Skip to content

Conversation

@mkrause
Copy link
Collaborator

@mkrause mkrause commented Dec 25, 2024

This PR:

  • Reworks and expands on the Banner component.
  • Removes the Alert component, which is a duplication.
  • Adds new "filled" variants of the success/error/info icons, for use in Banner.

Screenshot 2024-12-28 at 22 43 08

It also:

  • Fixes an issue with Storybook controls no longer working, a regression that happened with the splitting of tsconfig.json files.
  • Fixes some issues in the SegmentedControl and Tooltip components (which I needed for Banner).

TODO:

  • Need to get the right icons, the Figma has some icons that are not in the icon pack (e.g. filled success/error icons).

@mkrause mkrause self-assigned this Dec 25, 2024
@mkrause mkrause force-pushed the mkrause/241226-banner-update branch from a982cc4 to 9472d15 Compare December 26, 2024 01:15
@mkrause mkrause marked this pull request as ready for review December 26, 2024 01:16
@mkrause
Copy link
Collaborator Author

mkrause commented Dec 30, 2024

This apparently caused a regression in Chrome v121, which is the version used on Chromatic at the moment:

Screenshot 2024-12-30 at 18 25 29

I cannot reproduce this issue in any current browser, and just from the screenshots I have not been able to deduce the cause of this issue. It's not as simple as the icon being too large, it seems that the icon takes up the full width of the icon + title element, all the while the title text is hidden/collapsed. So the icon actually scales with the title text for some weird reason.

So this would need some investigation on an actual Chrome v121 instance. On the other hand, Chrome v121 was released in January 2024, so it's a year old by now and maybe we can ignore this as an old browser bug? Chromatic will update to a newer Chrome version in February 2025.

@mkrause mkrause requested a review from spli02 January 2, 2025 12:11
@mkrause mkrause merged commit 6d3ef40 into master Jan 2, 2025
3 checks passed
@mkrause mkrause deleted the mkrause/241226-banner-update branch January 2, 2025 17:02
@mkrause mkrause added this to the Baklava v1.0 milestone Feb 10, 2025
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