Pivot: JS Styling#5324
Pivot: JS Styling#5324JasonGore merged 11 commits intomicrosoft:masterfrom JasonGore:jg/convert-pivot-styling
Conversation
| ':after': { | ||
| color: 'transparent', | ||
| // TODO: how to represent this? | ||
| // content: attr(title) |
There was a problem hiding this comment.
I need to figure this out. If it's obvious or anybody knows, please let me know
| // TODO: not appearing in DOM / not working | ||
| ':before': { | ||
| boxSizing: 'border-box', | ||
| borderBottom: `2px solid ${palette.themePrimary}` |
There was a problem hiding this comment.
I need to figure this out too. The underline isn't appearing but I haven't looked too deeply into it yet. If it's obvious to anybody let me know
There was a problem hiding this comment.
display: block? or does position absolute assume display block?
There was a problem hiding this comment.
Turns out it's because there was no content. Content on line 50 needs to be `""` instead of `` (thanks @cliffkoh)
|
I need to make some changes to follow design toolkit per Ben's comments |
|
minor nit: can you make the focus border appear inside of the pivot render area? It looks to be a pixel outside. |
|
Link hover color removed and tab focus image updated to show border inside tab |
| selectors: { | ||
| '&:hover, &:focus': { | ||
| color: palette.black, | ||
| zIndex: 1 |
There was a problem hiding this comment.
minor nit here; zIndex=1 is really not ideal. I don't know why it's here. if possible, strongly question these ones. It means that it's possible this could overlap content that it wan't intended to overlap.
There was a problem hiding this comment.
Changed it and tested it and it still works fine, so I've committed it
| color: palette.themePrimary, | ||
| whiteSpace: 'nowrap' | ||
| }, | ||
| rootIsLarge && [classNames.rootIsLarge], |
There was a problem hiding this comment.
Yeah this evolved from something more complicated and I didn't unarray it
kenotron
left a comment
kenotron
left a comment
There was a problem hiding this comment.
This look good to me - though probably need designer to verify
| @@ -95,8 +95,6 @@ export interface IPivotStyleProps { | |||
| */ | |||
There was a problem hiding this comment.
Any reason not to do the Required<Pick<....>> & Pick<...> & {} pattern here like you are doing in TeachingBubble?
There was a problem hiding this comment.
No, same situation as yours. It was already in. 😆
There was a problem hiding this comment.
I had actually done the changes to TextField but that's in limbo for now.. I'll update and merge
|
Ben agreed the tab bordering should be there so it's staying in |
|
Yes tab border on key board focus |
* master: (151 commits) Css updates for card components (microsoft#5393) DatePicker MergeStyles step 2 - Converts scss to js styles (microsoft#5362) Update stale with 60 day comment and updated ignored label names. (microsoft#5388) replaced await for ie compat syntax - but still needs promise (microsoft#5387) Slider - mergestyle conversion (microsoft#5379) Applying package updates. Enabling publishing for charting and gridlayout. (microsoft#5378) TilesList: fadeOut overlay. (microsoft#5381) Pivot: JS Styling (microsoft#5324) ShimmeredDetailsList: wrapper for DetailsList with Shimmer. (microsoft#5374) dashboard-grid-layout improved example (microsoft#5373) Applying package updates. Addressing Issue microsoft#5165 - Using Customizer with Nav Component (microsoft#5361) Applying package updates. Address issue microsoft#5353 (microsoft#5359) Change log to trigger release (microsoft#5358) Fluent updates (microsoft#5357) TextField render value undefined as empty string (microsoft#5349) Applying package updates. DetailsColumn: css class changes to show gripper when hover on draggable columns (microsoft#5309) ...
7 participants
Pull request checklist
$ npm run changeDescription of changes
Convert Pivot to use JS styling.
Cursor navigation on tabbed elements now shows a highlighted border. This is consistent with non-tabbed controls.
Focus areas to test
Ensure JS styling has representation in snapshot output.
Microsoft Reviewers: Open in CodeFlow