-
Notifications
You must be signed in to change notification settings - Fork 55
docs(Toolbar): Prototype for custom toolbar #1541
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1541 +/- ##
=======================================
Coverage 71.53% 71.53%
=======================================
Files 847 847
Lines 6931 6931
Branches 1997 1997
=======================================
Hits 4958 4958
Misses 1967 1967
Partials 6 6 Continue to review full report at Codecov.
|
variables: { | ||
primary: true, | ||
}, | ||
}, |
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.
…hub.com/stardust-ui/react into feat/toolbar-custom-item
|
||
ctItemActiveBackground: siteVars.colorScheme.brand.backgroundHover1, | ||
ctItemActiveBackgroundOverlay: | ||
'linear-gradient(90deg,rgba(60,62,93,.6),rgba(60,62,93,0) 33%),linear-gradient(135deg,rgba(60,62,93,.6) 33%,rgba(60,62,93,0) 70%),linear-gradient(180deg,rgba(60,62,93,.6) 70%,rgba(60,62,93,0) 94%),linear-gradient(225deg,rgba(60,62,93,.6) 33%,rgba(60,62,93,0) 73%),linear-gradient(270deg,rgba(60,62,93,.6),rgba(60,62,93,0) 33%),linear-gradient(0deg,rgba(98,100,167,.75) 6%,rgba(98,100,167,0) 70%)', |
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.
Let's check before merge whether these colors exists in the color palette.
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.
Our plan is not to match redlines (for now), comment added.
...(v.isCtItemPrimary && { background: v.ctItemPrimaryBackground }), | ||
...(v.isCtItemIndicator && { padding: v.ctItemIndicatorPadding }), | ||
|
||
...(p.isFromKeyboard && { |
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.
Let's check with design whether these focusing styles are correct. And at least add comment that those should be changed if they should appear as in the other components.
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.
/cc @codepretty
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.
This rather shows how to change styles for focus than matches redlines.
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 agree, just if we can use the general getBorderFocusStyles
styles, we would be able to remove some styles... Let's not address this now until we are sure what is the expected state here...
}), | ||
|
||
...(v.isCtItemIconNoFill && { | ||
'& .ui-icon__filled': { |
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 here getIconFilledOrOutlineStyles.ts
? Or at least can we use [
& .${teamsIconClassNames.filled}]
and [
& .${teamsIconClassNames.outline}]
as selectors?
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.
Unfortunately there is currently no way how to access it from the outside of the library.
This is a bummer we should think about.
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.
Let's review comments before merging..
Adds a prototype for custom-styled toolbar