Nav: update to latest redlines #10008

phkuo · 2019-07-31T21:07:04Z

Pull request checklist

Addresses an existing issue: Fixes #0000
Include a change request file using $ yarn change

Description of changes

Tweaked the background colors for hover/selected in the Nav component, selected font styles, and fixed margins.

Old:

New:

Microsoft Reviewers: Open in CodeFlow

packages/office-ui-fabric-react/src/components/Nav/Nav.styles.ts

packages/fluent-theme/src/fluent/styles/Nav.styles.ts

change/@uifabric-styling-2019-07-31-14-05-43-phkuo-leftNavRedlines.json

size-auditor · 2019-07-31T22:16:36Z

Bundle test	Size (minified)	Diff from master
experiments-Button	94.889 kB	76 bytes
Dialog	191.305 kB	76 bytes
Dropdown	213.624 kB	76 bytes
Fabric	43.23 kB	76 bytes
Facepile	194.71 kB	76 bytes
FloatingPicker	222.01 kB	76 bytes
Foundation	42.049 kB	76 bytes
Grid	166.65 kB	76 bytes
GroupedList	119.7 kB	76 bytes
HoverCard	98.416 kB	76 bytes
Icon	49.201 kB	76 bytes
Icons	73.604 kB	76 bytes
Image	47.066 kB	76 bytes
Keytip	79.558 kB	76 bytes
KeytipLayer	101.867 kB	76 bytes
Label	42.836 kB	76 bytes
Tooltip	83.551 kB	76 bytes
DocumentCard	198.838 kB	76 bytes
DetailsList	208.397 kB	76 bytes
MarqueeSelection	75.467 kB	76 bytes
DatePicker	200.465 kB	76 bytes
ActivityItem	80.15 kB	76 bytes
Announced	42.927 kB	76 bytes
Breadcrumb	183.262 kB	76 bytes
Button	178.419 kB	76 bytes
Calendar	144.319 kB	76 bytes
Callout	83.431 kB	76 bytes
Check	52.328 kB	76 bytes
Checkbox	63.884 kB	76 bytes
ChoiceGroup	61.618 kB	76 bytes
ChoiceGroupOption	57.247 kB	76 bytes
Coachmark	96.314 kB	76 bytes
ColorPicker	83.437 kB	76 bytes
ComboBox	224.498 kB	76 bytes
CommandBar	185.305 kB	76 bytes
ContextualMenu	147.112 kB	76 bytes
Link	56.208 kB	76 bytes
Layer	49.494 kB	76 bytes
MessageBar	174.14 kB	76 bytes
Spinner	55.222 kB	76 bytes
ScrollablePane	62.336 kB	76 bytes
SelectedItemsList	212.253 kB	76 bytes
Shimmer	58.403 kB	76 bytes
ShimmeredDetailsList	218.969 kB	76 bytes
Slider	64.968 kB	76 bytes
SpinButton	177.891 kB	76 bytes
Stack	45.053 kB	76 bytes
ProgressIndicator	43.519 kB	76 bytes
Styling	44.397 kB	76 bytes
SwatchColorPicker	176.48 kB	76 bytes
TeachingBubble	177.355 kB	76 bytes
Text	41.027 kB	76 bytes
TextField	75.857 kB	76 bytes
Toggle	62.305 kB	76 bytes
Rating	73.022 kB	76 bytes
SearchBox	172.433 kB	76 bytes
Persona	116.011 kB	76 bytes
Modal	92.349 kB	76 bytes
PositioningContainer	75.796 kB	76 bytes
Overlay	54.247 kB	76 bytes
Panel	183.306 kB	76 bytes
Pivot	171.957 kB	76 bytes
Pickers	262.417 kB	76 bytes
PersonaPresence	66.035 kB	76 bytes
PersonaCoin	116.011 kB	76 bytes
Nav	173.062 kB	54 bytes

Exceeds Tolerance Exceeds Baseline Below Baseline 1 kB = 1000 bytes

change/@uifabric-styling-2019-07-31-14-05-43-phkuo-leftNavRedlines.json

Vitalius1

Thanks for addressing the feedback

phkuo · 2019-08-01T23:49:30Z

@dzearing need your review
@cliffkoh can you bypass?

phkuo · 2019-08-02T21:24:31Z

@dzearing @dzearing @dzearing

dzearing · 2019-08-03T06:35:24Z

The build is still failing

dzearing · 2019-08-03T06:38:57Z

Screenshot?

phkuo · 2019-08-05T18:46:41Z

Restarted to get screener to work.
You wouldn't be able to see anything in screenshots, I just changed the background shades to be one shade darker. Nothing terribly interesting, but it SHOULD fail screener.

phkuo · 2019-08-05T21:28:41Z

@Vitalius1 @betrue-final-final do you know why Screener isn't responding?

Vitalius1 · 2019-08-06T02:29:24Z

@phkuo The CI is failing on a snapshot that need to be updated so this is why it never starts the Screener. 😄

phkuo · 2019-08-06T21:55:04Z

This is failing the accessibility test. Putting this PR on hold while I get some questions answered from design.

msft-github-bot · 2019-08-09T01:51:28Z

Hello @phkuo!

Because this pull request has the Auto Merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msft-github-bot`) and give me an instruction to get started! Learn more here.

phkuo · 2019-08-09T01:51:32Z

This will need to be ported to Fabric 6 as well.

packages/office-ui-fabric-react/src/components/Nav/Nav.styles.ts

Vitalius1

Don’t you have to run these script now that you add 2 slots?

Also, looks like you need another round of updating snapshots 😄

phkuo · 2019-08-13T20:35:38Z

@Vitalius1 @natalieethell running update-sass-theme-files doesn't do anything, oddly. I don't have time to debug it, but Natalie could you take a look? I'm on call this week.

phkuo · 2019-08-13T22:14:35Z

@dzearing pokepoke

packages/fluent-theme/src/fluent/styles/Nav.styles.ts

dzearing · 2019-08-13T23:04:24Z

packages/fluent-theme/src/fluent/styles/Nav.styles.ts

+          '&:hover': {
+            backgroundColor: semanticColors.bodyBackgroundHovered
+          },
+          '$compositeLink:hover &': {


The $ syntax is not supported any longer. Please don't use it.

packages/mdl2-theme/src/mdl2/styles/Nav.styles.ts

packages/office-ui-fabric-react/src/components/Nav/Nav.styles.ts

dzearing

Fix styling comments.

natalieethell · 2019-08-13T23:44:37Z

@phkuo it looks like you already have those colors in _semanticSlots.scss from your PR 6 days ago:

https://github.com/OfficeDev/office-ui-fabric-react/pull/10093/files#diff-3ebaa175fa5a19693419fd7fabbd751cR3-R4

$bodyBackgroundHoveredColor: '[theme:bodyBackgroundHovered, default: #f3f2f1]'; 
$bodyBackgroundCheckedColor: '[theme:bodyBackgroundChecked, default: #edebe9]';

phkuo · 2019-08-19T17:50:37Z

@dzearing ready for re-review

msft-github-bot · 2019-08-20T12:31:40Z

🎉@uifabric/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

msft-github-bot · 2019-08-20T12:31:41Z

🎉[email protected] has been released which incorporates this pull request.:tada:

Handy links:

msft-github-bot · 2019-08-20T12:31:42Z

🎉@uifabric/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

msft-github-bot · 2019-08-20T12:31:48Z

🎉@uifabric/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

…t/5.79.1 (#14933) * Nav: update to latest redlines * add change log Co-authored-by: Jon Schectman <[email protected]>

…e-ui-fabric-react_v5.79.1 to hotfix/5.135.6 (#22162) * Port #14933 (Port #10008 to update Nav to latest redlines) from office-ui-fabric-react_v5.79.1 to hotfix/5.135.6 * add commit log

phkuo added 3 commits July 30, 2019 15:22

first

47fe443

add nav style reversion to MDL2 theme package

7779a34

Change files

9da853f

phkuo requested review from ecraig12345 and Vitalius1 July 31, 2019 21:07

msft-github-bot assigned ecraig12345 Jul 31, 2019

Vitalius1 requested changes Jul 31, 2019

View reviewed changes

packages/office-ui-fabric-react/src/components/Nav/Nav.styles.ts Outdated Show resolved Hide resolved

packages/fluent-theme/src/fluent/styles/Nav.styles.ts Outdated Show resolved Hide resolved

msft-github-bot added the Needs: Author Feedback label Jul 31, 2019

Vitalius1 reviewed Jul 31, 2019

View reviewed changes

change/@uifabric-styling-2019-07-31-14-05-43-phkuo-leftNavRedlines.json Outdated Show resolved Hide resolved

update snapshots and API

cb9ff31

msft-github-bot removed the Needs: Author Feedback label Jul 31, 2019

code review feedback

4353d6f

phkuo requested a review from dzearing as a code owner July 31, 2019 22:01

Vitalius1 reviewed Jul 31, 2019

View reviewed changes

change/@uifabric-styling-2019-07-31-14-05-43-phkuo-leftNavRedlines.json Outdated Show resolved Hide resolved

Vitalius1 approved these changes Jul 31, 2019

View reviewed changes

update

47e8885

ecraig12345 added the Component: Nav label Aug 1, 2019

phkuo closed this Aug 5, 2019

phkuo reopened this Aug 5, 2019

phkuo added 2 commits August 6, 2019 14:00

Merge branch 'master' into phkuo/leftNavRedlines

66c4d8b

more snapshots

0e20f8d

Vitalius1 reviewed Aug 9, 2019

View reviewed changes

packages/office-ui-fabric-react/src/components/Nav/Nav.styles.ts Show resolved Hide resolved

Vitalius1 approved these changes Aug 9, 2019

View reviewed changes

phkuo added 2 commits August 13, 2019 10:59

Merge branch 'master' into phkuo/leftNavRedlines

5625b45

changes

5c765a1

fix tslint error

2df746d

dzearing reviewed Aug 13, 2019

View reviewed changes

packages/fluent-theme/src/fluent/styles/Nav.styles.ts Outdated Show resolved Hide resolved

dzearing reviewed Aug 13, 2019

View reviewed changes

packages/mdl2-theme/src/mdl2/styles/Nav.styles.ts Show resolved Hide resolved

dzearing reviewed Aug 13, 2019

View reviewed changes

packages/office-ui-fabric-react/src/components/Nav/Nav.styles.ts Outdated Show resolved Hide resolved

dzearing requested changes Aug 13, 2019

View reviewed changes

msft-github-bot added the Needs: Author Feedback label Aug 13, 2019

msft-github-bot removed the Needs: Author Feedback label Aug 14, 2019

Fixes

b7cbd30

dzearing approved these changes Aug 19, 2019

View reviewed changes

msft-github-bot merged commit f0097f8 into microsoft:master Aug 19, 2019

Phoebe-0319 mentioned this pull request Sep 8, 2020

Port #10008 to update Nav to latest redlines in office-ui-fabric-react/5.79.1 #14933

Merged

2 tasks

joschect added a commit that referenced this pull request Oct 13, 2020

Port #10008 to update Nav to latest redlines in office-ui-fabric-reac…

e27273d

…t/5.79.1 (#14933) * Nav: update to latest redlines * add change log Co-authored-by: Jon Schectman <[email protected]>

Phoebe-0319 mentioned this pull request Feb 10, 2022

Add SBom generation (5.79.1) #21676

Merged

Phoebe-0319 mentioned this pull request Mar 19, 2022

Port #14933 (Port #10008 to update Nav to latest redlines) from office-ui-fabric-react_v5.79.1 to hotfix/5.135.6 #22162

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Nav: update to latest redlines #10008

Nav: update to latest redlines #10008

phkuo commented Jul 31, 2019 •

edited

Loading

size-auditor bot commented Jul 31, 2019 •

edited

Loading

Vitalius1 left a comment

phkuo commented Aug 1, 2019

phkuo commented Aug 2, 2019

dzearing commented Aug 3, 2019

dzearing commented Aug 3, 2019

phkuo commented Aug 5, 2019

phkuo commented Aug 5, 2019

Vitalius1 commented Aug 6, 2019

phkuo commented Aug 6, 2019

msft-github-bot commented Aug 9, 2019

phkuo commented Aug 9, 2019

Vitalius1 left a comment

phkuo commented Aug 13, 2019

phkuo commented Aug 13, 2019

dzearing Aug 13, 2019

dzearing left a comment

natalieethell commented Aug 13, 2019

phkuo commented Aug 19, 2019

msft-github-bot commented Aug 20, 2019

msft-github-bot commented Aug 20, 2019

msft-github-bot commented Aug 20, 2019

msft-github-bot commented Aug 20, 2019

Nav: update to latest redlines #10008

Nav: update to latest redlines #10008

Conversation

phkuo commented Jul 31, 2019 • edited Loading

Pull request checklist

Description of changes

Microsoft Reviewers: Open in CodeFlow

size-auditor bot commented Jul 31, 2019 • edited Loading

Vitalius1 left a comment

Choose a reason for hiding this comment

phkuo commented Aug 1, 2019

phkuo commented Aug 2, 2019

dzearing commented Aug 3, 2019

dzearing commented Aug 3, 2019

phkuo commented Aug 5, 2019

phkuo commented Aug 5, 2019

Vitalius1 commented Aug 6, 2019

phkuo commented Aug 6, 2019

msft-github-bot commented Aug 9, 2019

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msft-github-bot) and give me an instruction to get started! Learn more here.

phkuo commented Aug 9, 2019

Vitalius1 left a comment

Choose a reason for hiding this comment

phkuo commented Aug 13, 2019

phkuo commented Aug 13, 2019

dzearing Aug 13, 2019

Choose a reason for hiding this comment

dzearing left a comment

Choose a reason for hiding this comment

natalieethell commented Aug 13, 2019

phkuo commented Aug 19, 2019

msft-github-bot commented Aug 20, 2019

msft-github-bot commented Aug 20, 2019

msft-github-bot commented Aug 20, 2019

msft-github-bot commented Aug 20, 2019

phkuo commented Jul 31, 2019 •

edited

Loading

size-auditor bot commented Jul 31, 2019 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msft-github-bot`) and give me an instruction to get started! Learn more here.