-
Notifications
You must be signed in to change notification settings - Fork 13
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
Redesign notifications #619
Conversation
- SearchFilterPanelHeader now uses DrawerHeader. - NotificationHeader now uses DrawerHeader. - Add NotificationList component, which should wrap notifications. - Increase height of tab menu to 40px. - Update design of notifications, including a large demo story as designed by Chris. - Update CircledIcon stories to use Stena icons (except for spinner). - Remove iconSize prop from CircledIcon. Can now only use predefined size variants, which sets both background and icon size.
Size Change: 0 B Total Size: 352 kB ℹ️ View Unchanged
|
const getIconSize = (size: CircledIconSizeVariant): number => { | ||
switch (size) { | ||
case "small": | ||
return 16; | ||
case "medium": | ||
return 20; | ||
default: | ||
return exhaustSwitchCaseElseThrow(size); | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is background size a string, when this is a number? Use number everywhere!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd argue the opposite.
Icon takes number and then converts it to rem (40 becomes 4.0rem).
Box can take a number, but just adds "px" suffix (40 becomes 40px).
Unit should be explicit so that it is clear where the UI should scale with accessible font size, and where it should not.
Regardless, that is not within the scope of notifications :)
One could argue that the icon should not scale with accessible font size, but that would affect many many components. I'll take it up with Chris and Anton.
…ys-stuff # Conflicts: # packages/filter/src/features/search-filter/components/SearchFilterPanelHeader.tsx
Version
Published prerelease version:
v19.0.0-next.15
Changelog
💥 Breaking Change
@stenajs-webui/core
,@stenajs-webui/elements
,@stenajs-webui/panels
🐛 Bug Fix
@stenajs-webui/elements
,@stenajs-webui/filter
,@stenajs-webui/modal
,@stenajs-webui/panels
@stenajs-webui/elements
,@stenajs-webui/panels
@stenajs-webui/filter
,@stenajs-webui/modal
@stenajs-webui/panels
@stenajs-webui/elements
,@stenajs-webui/theme
@stenajs-webui/elements
,@stenajs-webui/grid-export
,@stenajs-webui/panels
@stenajs-webui/forms
,@stenajs-webui/panels
,@stenajs-webui/theme
@stenajs-webui/core
,@stenajs-webui/elements
,@stenajs-webui/forms
,@stenajs-webui/panels
,@stenajs-webui/theme
@stenajs-webui/elements
@stenajs-webui/calendar
,@stenajs-webui/elements
,@stenajs-webui/filter
,@stenajs-webui/forms
,@stenajs-webui/grid-export
,@stenajs-webui/grid
,@stenajs-webui/panels
,@stenajs-webui/theme
@stenajs-webui/core
,@stenajs-webui/elements
,@stenajs-webui/filter
,@stenajs-webui/modal
,@stenajs-webui/panels
,@stenajs-webui/theme
@stenajs-webui/core
,@stenajs-webui/elements
,@stenajs-webui/panels
,@stenajs-webui/theme
@stenajs-webui/theme
@stenajs-webui/filter
@stenajs-webui/elements
,@stenajs-webui/panels
,@stenajs-webui/theme
,@stenajs-webui/tooltip
@stenajs-webui/elements
,@stenajs-webui/modal
,@stenajs-webui/panels
next
Authors: 4