-
Notifications
You must be signed in to change notification settings - Fork 16
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
AppBar: Add support for custom branding #792
Conversation
This pull request has been linked to Shortcut Story #344453: Whitelabel issue: icons and other elements are not respecting the app bar text color. |
Pull Request Test Coverage Report for Build 6575744574
💛 - Coveralls |
Visit the preview URL for this PR (updated for commit a2dc022): https://cartodb-fb-storybook-react-dev--pr792-feature-appbar-b-xqap4mfy.web.app (expires Thu, 26 Oct 2023 14:11:24 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 517cc4d31d7e09cf277774e034094b67c301cd4c |
@@ -52,7 +54,15 @@ const rows = [ | |||
const options = { | |||
title: 'Molecules/Table', | |||
component: Table, | |||
argTypes: {}, | |||
argTypes: { |
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.
Offtopic... story improved as a design request
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.
LGTM
}) | ||
})); | ||
|
||
export default function BurguerMenu({ onClickMenu, iconColor }) { |
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.
minor, shouldn't it be burger instead of burguer ?
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.
Nice catch, fixed, thanks!
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 looks yummy in any case
@@ -9,6 +9,8 @@ export type AppBarTypeMap<D extends React.ElementType<any> = 'header'> = MuiAppB | |||
secondaryText?: React.ReactNode; | |||
onClickMenu?: (event: React.MouseEvent) => void; | |||
showBurgerMenu?: boolean; | |||
backgroundColor?: string; |
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.
Oks, I see the gap, 👍🏻
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.
👍🏻 Nice work
Description
Shortcut: https://app.shortcut.com/cartoteam/story/344453/whitelabel-issue-icons-and-other-elements-are-not-respecting-the-app-bar-text-color
[sc-344453]
Add basic support for colors customization in AppBar core components.