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.

Sign up for GitHub

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

Jump to bottom

feat(ui-shell): add theming support #8579

Merged
merged 20 commits into from
Jun 1, 2021
Merged

feat(ui-shell): add theming support #8579

merged 20 commits into from
Jun 1, 2021

Conversation

andreancardona
Copy link
Contributor

@andreancardona andreancardona commented May 4, 2021 •
edited by jnm2377
Loading

Closes #8409
Closes #8102

This is the first step in creating our UI Shell Theme

  • Update _header.scss files to match most recent styles
  • Update _side-nav.scss files to match most recent styles
  • Update _switcher.scss files to match most recent styles
  • Update _header-panel.scss files to match most recent styles
  • Update _navigation-menu.scss files to match most recent styles
  • Update _product-switcher.scss files to match most recent styles

Deprecated the following components that did not have a design spec (per discussion with design):

  • HeaderPanel + accompanying styles
  • SideNavDetails + accompanying styles
  • SideNavFooter + accompanying styles
  • SideNavHeader + accompanying styles
  • SideNavSwitcher + accompanying styles
  • navigation-menu styles (which did not have a component built out)
  • custom UI-shell theme tokens

@andreancardona andreancardona requested a review from jnm2377 May 4, 2021 22:18
@andreancardona andreancardona requested a review from a team as a code owner May 4, 2021 22:18
@andreancardona andreancardona requested a review from tw15egan May 4, 2021 22:18
@andreancardona andreancardona marked this pull request as draft May 4, 2021 22:18
@netlify
Copy link

netlify bot commented May 4, 2021 •
edited
Loading

Deploy Preview for carbon-elements ready!

Built with commit 702135f

https://deploy-preview-8579--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented May 4, 2021 •
edited
Loading

Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables with commit 702135f

https://deploy-preview-8579--carbon-components-react.netlify.app

@jnm2377 jnm2377 requested a review from aagonzales May 12, 2021 21:08
@jnm2377 jnm2377 changed the title [DRAFT] feat(ui-shell): updated ui shell header styles feat(ui-shell): updated ui shell header styles May 12, 2021
@jnm2377 jnm2377 changed the title feat(ui-shell): updated ui shell header styles feat(ui-shell): add theming support May 12, 2021
@jnm2377 jnm2377 marked this pull request as ready for review May 12, 2021 21:13
aagonzales
aagonzales reviewed May 14, 2021
View reviewed changes
Copy link
Member

@aagonzales aagonzales left a comment

Choose a reason for hiding this comment

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

Its looking good! Just a few changes.

The selected state for header actions isn't showing when the panel is open
image

The menu items in the header should be using $layer for its background color.
image

The icon in a selected item in the left nav should be using $icon-01/$icon-primary
image

@andreancardona andreancardona mentioned this pull request May 17, 2021
Closed
94 tasks
@jnm2377 jnm2377 requested a review from aagonzales May 18, 2021 22:35
@andreancardona andreancardona mentioned this pull request May 19, 2021
Closed
87 tasks
@tw15egan
Copy link
Collaborator

This looks awesome! I think the only issue I'm seeing is that the bx--content class doesn't set text color, so the text gets washed out in dark themes:

Screen Shot 2021-05-20 at 12 26 54 PM

I was able to fix this by adding color: $text-primary; on L19 in _content.scss

@andreancardona
Copy link
Contributor Author

@tw15egan thanks for catching that - just updated :)

@tw15egan
Copy link
Collaborator

Do we need to hold off on merging this until we have a v11 branch? Seems like this could cause unexpected changes to teams using the UI Shell in g10 / white themes and suddenly having a light header. I believe they would need to be using custom properties to revert back to the dark header, which could be a breaking change to enable for some teams

@andreancardona
Copy link
Contributor Author

d off on merging this until we have a v11 branch? Seems like this could cause unexpected changes to teams using the UI Shell in g10 / white themes and suddenly having a light header. I believe they

Yep I agree we should hold off..

@jnm2377
Copy link
Contributor

jnm2377 commented May 25, 2021

Should we change the base to next now that we have a next branch?

@tw15egan
Copy link
Collaborator

@jnm2377 yeah let's go for it 🎸

@tw15egan tw15egan changed the base branch from main to next May 25, 2021 19:41
tay1orjones
tay1orjones approved these changes May 25, 2021
View reviewed changes
Copy link
Member

@tay1orjones tay1orjones left a comment

Choose a reason for hiding this comment

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

One change I suggested but looks good otherwise!

@andreancardona
Copy link
Contributor Author

One change I suggested but looks good otherwise!

thanks for catching that!

@tw15egan tw15egan merged commit f79b628 into carbon-design-system:next Jun 1, 2021
emyarod pushed a commit to emyarod/carbon that referenced this pull request Jun 10, 2021
@andreancardona @jnm2377
@tw15egan @tay1orjones @emyarod
feat(ui-shell): add theming support (carbon-design-system#8579)
73d34ad 
* feat(ui-shell): updated ui shell header styles

* feat(ui-shell): updated side nav styles

* feat(ui-shell): updated header-panel & switcher styles

* feat(ui-shell): _side-nav.scss token update

* feat(ui-shell): updated and deprecate styles

* chore: update deprecated component tests

* fix: test warning path

* fix: design feedback changes

* feat(ui-shell): update depcrecation test warning

* fix(content): added content color with updated styles

* Update packages/react/src/components/UIShell/HeaderPanel.js

Co-authored-by: Taylor Jones <[email protected]>

* Update HeaderPanel.js

Co-authored-by: Josefina Mancilla <[email protected]>
Co-authored-by: Josefina Mancilla <[email protected]>
Co-authored-by: TJ Egan <[email protected]>
Co-authored-by: Taylor Jones <[email protected]>
This was referenced Jul 30, 2021
Closed
Closed
Closed
Closed
Closed
@metonym metonym mentioned this pull request Oct 4, 2021
Closed
@metonym metonym mentioned this pull request Nov 8, 2021
Open
@jnm2377
Copy link
Contributor

jnm2377 commented Mar 15, 2022

Just for tracking purposes, this was never merged into main. After this PR was included in /next we started working on the new styles package for v11, and the new tokens were then added to UI shell stylesheets there in this PR #9195

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jnm2377 jnm2377 jnm2377 approved these changes

@tw15egan tw15egan tw15egan approved these changes

@aagonzales aagonzales aagonzales approved these changes

@tay1orjones tay1orjones tay1orjones approved these changes

Assignees
No one assigned
Labels
status: ready to merge 🎉
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Theme the UI Shell [Design] UI Shell - new color token spec
5 participants
@andreancardona @tw15egan @jnm2377 @tay1orjones @aagonzales