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
Add banner bar for custom parameters #7045
Conversation
import NotificationsBar from '/imports/ui/components/notifications-bar/component'; | ||
import { styles } from './styles'; | ||
|
||
const BannerBar = ({ text, color }) => { |
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.
const BannerBar = ({ text, color }) => !text ? null : (
...
)```
import BannerComponent from './component'; | ||
|
||
export default withTracker(() => ({ | ||
color: Session.get('bannerColor') || undefined, |
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.
Does Session.get
returns something other than undefined
for undefined keys? 🤔
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 did that to trigger the default props, session.get
return null
when no there the value.
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.
Gotcha but that's not implicit. I would suggest to remove the defaultProps
and make them required. And then swap the undefined
for the default values.
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.
The colour of the other notification bars seems to be broken. The following image shows the BannerBar and the bar that shows the meeting duration under it. The text is there, but the font colour is the same as the background.
The second bar should look like this:
The breakout room duration bar also has the same issue.
Closes #6939.