Skip to content
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

Semantic slot updates nov2018 #7176

Merged
merged 13 commits into from
Nov 27, 2018

Conversation

bennettclark
Copy link
Contributor

@bennettclark bennettclark commented Nov 20, 2018

Pull request checklist

  • Addresses an existing issue: Fixes #0000
  • Include a change request file using $ npm run change

Description of changes

Updated semantic slot list and variant logic with new slots and values from designers. The new snapshots are from the color value updates. Also, api files needed to be updated for the changed packages.

It's a lot of files to look through, if you're looking for the files that were modified to make all these new snapshots, you'll want to look at:
ISemanticColors.ts
ISemanticTextColors.ts
theme.ts
_semanticSlots.scss
variants.ts

Also various components were updated to change disabledBodyText to the new disabledBodySubtext slot (disabledBodyText is only for text color, while disabledBodySubtext is used for other disabled elements like borders), per Ben and Sirena's direction.

Microsoft Reviewers: Open in CodeFlow

@bennettclark
Copy link
Contributor Author

@betrue-final-final - I believe you're familiar with these semantic slot updates, can you please review the screener test for this PR?

@betrue-final-final
Copy link
Member

It appears that the disabled icon slot is wrong. The disabled check, toggle, should be #a6a6a6 and not #c8c8c8, and the toggle thumb looks like it got lighter in code, but the change was subtle.

@aneeshack4
Copy link
Contributor

@bennettclark It looks like there are some regressions in screener - is there a plan to fix these?

@bennettclark
Copy link
Contributor Author

@aneeshack4 @betrue-final-final I'm waiting to hear back from Sirena to confirm that the change of disabledBodyText from #c8c8c8 to #a6a6a6 wasn't a mistake or typo, if she confirms that was the value she was given then I'll be starting a convo with Ben and Sirena to get that sorted out

@betrue-final-final
Copy link
Member

The text change was correct but the icons and actual check box need to remain #c8c8c8. This was something that Sirena, myself and Janet worked out before.

@bennettclark
Copy link
Contributor Author

@betrue-final-final - can you take a look at the screener results now? It looks like the changes are all on the text color now, which I believe you said was correct. (Also, I'm not sure what the change is on the toggle result.)

@bennettclark
Copy link
Contributor Author

@srideshpande @dzearing @aneeshack4 - Ben has signed off on these design changes and all checks have passed, so as soon as you have a chance to take a look and sign off on this PR that would be much appreciated!

@aneeshack4
Copy link
Contributor

LGTM @srideshpande @dzearing do y'all have anything to add? I think we need 1 code owner review.

Copy link
Contributor

@srideshpande srideshpande left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@srideshpande
Copy link
Contributor

Approving for the choice group changes.

@aneeshack4
Copy link
Contributor

@bennettclark looks like there's a merge conflict in Toggle.styles.ts
Also this needs code owner review - not sure how to tag OfficeDev/uifabric-calendar so pinging!

Copy link
Contributor

@lorejoh12 lorejoh12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approving for DatePicker, it's just a snapshot change there

@dzearing dzearing merged commit 3f75dd9 into microsoft:master Nov 27, 2018
bennettclark added a commit to bennettclark/office-ui-fabric-react that referenced this pull request Nov 27, 2018
cliffkoh pushed a commit that referenced this pull request Nov 28, 2018
* hand cherry picked updates from PR #7176

* change logs

* change single quotes back to double quotes

* add back deprecated slot for backwards compatibility

* newlines at end of files

* update disabledBodySubtext for non-text elements that used disabledBodyText
@msft-github-bot
Copy link
Contributor

🎉@uifabric/styling@v6.37.0 has been released which incorporates this pull request.:tada:

Handy Links:

@msft-github-bot
Copy link
Contributor

🎉@uifabric/fluent-theme@v0.9.0 has been released which incorporates this pull request.:tada:

Handy Links:

@msft-github-bot
Copy link
Contributor

🎉@uifabric/experiments@v6.43.0 has been released which incorporates this pull request.:tada:

Handy Links:

@msft-github-bot
Copy link
Contributor

🎉@uifabric/variants@v6.13.0 has been released which incorporates this pull request.:tada:

Handy Links:

@msft-github-bot
Copy link
Contributor

🎉office-ui-fabric-react@v6.108.0 has been released which incorporates this pull request.:tada:

Handy Links:

@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants