-
-
Notifications
You must be signed in to change notification settings - Fork 827
Conversation
...and add support for compound design token icons to TabbedView, changing all the other icons over while we're at it.
if (tab.icon && typeof tab.icon === "object") { | ||
tabIcon = tab.icon; | ||
} else if (typeof tab.icon === "string") { | ||
tabIcon = <span className={`mx_TabbedView_maskedIcon ${tab.icon}`} />; | ||
} |
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 does only one of these paths have the truthy check? previously the string path had a truthy check
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.
It would have to be the empty string to be of type string but still falsy, but I can add it around the whole lot easily enough
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.
null
would also hit this edge, no?
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.
null
is of type object
so it wouldn't match either case.
import { Icon as UserProfileIcon } from "@vector-im/compound-design-tokens/icons/user-profile.svg"; | ||
import { Icon as DevicesIcon } from "@vector-im/compound-design-tokens/icons/devices.svg"; | ||
import { Icon as VisibilityOnIcon } from "@vector-im/compound-design-tokens/icons/visibility-on.svg"; | ||
import { Icon as NotificationsIcon } from "@vector-im/compound-design-tokens/icons/notifications.svg"; | ||
import { Icon as PreferencesIcon } from "@vector-im/compound-design-tokens/icons/preferences.svg"; | ||
import { Icon as KeyboardIcon } from "@vector-im/compound-design-tokens/icons/keyboard.svg"; | ||
import { Icon as SidebarIcon } from "@vector-im/compound-design-tokens/icons/sidebar.svg"; | ||
import { Icon as MicOnIcon } from "@vector-im/compound-design-tokens/icons/mic-on.svg"; | ||
import { Icon as LockIcon } from "@vector-im/compound-design-tokens/icons/lock.svg"; | ||
import { Icon as LabsIcon } from "@vector-im/compound-design-tokens/icons/labs.svg"; | ||
import { Icon as BlockIcon } from "@vector-im/compound-design-tokens/icons/block.svg"; | ||
import { Icon as HelpIcon } from "@vector-im/compound-design-tokens/icons/help.svg"; |
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.
Can we use the new CDT exports please in assets/web, they ensure our snapshots include the svg which means we can detect changes in icons even if we're missing screenshot tests
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.
ah, yes, that's better
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.
Indeed, we should probably eslint ban the source svgs
...and add support for compound design token icons to TabbedView, changing all the other icons over while we're at it.
To go with #12841
Checklist
public
/exported
symbols have accurate TSDoc documentation.