-
Notifications
You must be signed in to change notification settings - Fork 53
Conversation
packages/react/src/themes/teams-dark/components/Toolbar/toolbarVariables.ts
Outdated
Show resolved
Hide resolved
import { ToolbarVariables } from '../../../teams/components/Toolbar/toolbarVariables' | ||
|
||
export default (siteVars: any): Partial<ToolbarVariables> => ({ | ||
foreground: siteVars.colors.white, |
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.
just the thought unrelated to this PR - really think that we need to introduce siteVariables
's types for Teams theme - otherwise it might become to be pretty complicated story to maintain all this, especially given that name of the variable is not fixed on the client side
Codecov Report
@@ Coverage Diff @@
## master #1408 +/- ##
==========================================
- Coverage 73.57% 73.49% -0.09%
==========================================
Files 787 797 +10
Lines 5885 5949 +64
Branches 1734 1746 +12
==========================================
+ Hits 4330 4372 +42
- Misses 1549 1571 +22
Partials 6 6
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #1408 +/- ##
==========================================
- Coverage 73.49% 73.27% -0.22%
==========================================
Files 793 802 +9
Lines 5949 6025 +76
Branches 1754 1775 +21
==========================================
+ Hits 4372 4415 +43
- Misses 1571 1604 +33
Partials 6 6
Continue to review full report at Codecov.
|
'backgroundFocus', | ||
'borderFocus', | ||
|
||
'foregroundDisabled1', |
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.
nit: we may want to extend the color scheme, and use the foregroundDisabled1
for the foregroundDisabled
, so it would be more user friendly, but then it may raise a problem because the user will think this one is acttually the foregroundDisabled
from the color scheme... Let's just think about this and decide together whether we want that.
Basic
Toolbar
implementationToolbar currently supports three kinds of items:
ToolbarItem
- the icon button in the toolbar,ToolbarDivider
- visual separator between items,ToolbarRadiogroup
- logical grouping ofToolbarItem
(can also containToolbarDivider
).See https://www.w3.org/TR/wai-aria-practices/examples/toolbar/toolbar.html for details on accessibility.
To be handled in separate tasks: