Skip to content

Conversation

@crnkovic
Copy link
Contributor

@crnkovic crnkovic commented Nov 2, 2022

Summary

https://app.clickup.com/t/315h46e

Screenshot 2022-11-02 at 15 32 40

Screenshot 2022-11-02 at 15 32 57

Screenshot 2022-11-02 at 15 33 15

Checklist

  • I checked my UI changes against the design and there are no notable differences
  • I checked my UI changes for any responsiveness issues
  • I checked my (code) changes for obvious issues, debug statements and commented code
  • I provided a screenshot of my changes to the component (if applicable)
  • I regenerated the icons.html file and checked if my newly added icon is shown correctly (if necessary)
  • I added an explanation on how to use the component to the readme (if necessary)
  • Documentation (if necessary)
  • Tests (if necessary)
  • Ready to be merged

@crnkovic crnkovic marked this pull request as ready for review November 2, 2022 14:35
Copy link
Member

@ItsANameToo ItsANameToo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some remarks:

  • can you update the mobile alert styling to match the toasts? those seem to be identical now in the styleguide, while alerts are currently using a smaller icon (14px instead of 16px)
  • can you add the spinner handling to the usage guide so it's clear how to make it appear/hide when needed
  • can the titles be made configurable like the alerts have? so we can set a custom title if needed instead of having Success for a success toast

@ItsANameToo ItsANameToo marked this pull request as draft November 3, 2022 10:50
@crnkovic crnkovic marked this pull request as ready for review November 7, 2022 10:59
@ItsANameToo ItsANameToo merged commit 24d3272 into main Nov 9, 2022
@ItsANameToo ItsANameToo deleted the refactor/toast-design branch November 9, 2022 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants