Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Theming the toast #17878

Closed
ulisesmac opened this issue Nov 10, 2023 · 2 comments · Fixed by #17918
Closed

Theming the toast #17878

ulisesmac opened this issue Nov 10, 2023 · 2 comments · Fixed by #17918
Assignees

Comments

@ulisesmac
Copy link
Contributor

Problem

The current toast implementation needs an :icon-color which needs to be provided along with the current app theme.
The toast should read the theme as other components in the app: using our theme HOC.

Example in status-im2.contexts.wallet.edit-account.view:

(rf/dispatch [:toasts/upsert
              {:id         :edit-account
               :icon       :i/correct
               :icon-color (colors/resolve-color :success theme)
               :text       (i18n/label message)}])

Acceptance Criteria

App is working as expected and the toast is showing in the expected ways for every screen,

Notes

Related issue:

@codemaster115 codemaster115 self-assigned this Nov 15, 2023
@codemaster115
Copy link
Contributor

@ulisesmac I don't think the component needs :icon-color property.
But by following the figma design, it requires to have :type property to match proper icons

@codemaster115 codemaster115 linked a pull request Nov 15, 2023 that will close this issue
@ulisesmac
Copy link
Contributor Author

ulisesmac commented Nov 16, 2023

@ulisesmac I don't think the component needs :icon-color property. But by following the figma design, it requires to have :type property to match proper icons

@codemaster115 Interesting.

This issue could be a good opportunity to properly implement the toast 👍 then.
Also

is about refactoring the implementation.

So we could do everything in a single issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants