feat(masthead): spacing style regression fix and maintainability refactor #11252
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
@carbon/styles package's utilities default export only forwards the em() and rem() functions, but we need to-rem().
The v2 masthead has feature support for everything the Cloud masthead needs.
jkaeser
added
package: styles
|
Deploy preview created for package Built with commit: 98649808c5577552ad9e79592138d7fba1767562 |
|
Deploy preview created for package Built with commit: 98649808c5577552ad9e79592138d7fba1767562 |
jkaeser
requested review from
ariellalgilmore and
annawen1
and removed request for
a team
jkaeser
requested review from
m4olivei,
bruno-amorim,
marcelojcs and
andy-blum
kennylam
approved these changes
Jan 5, 2024
m4olivei
suggested changes
Jan 9, 2024
m4olivei
approved these changes
Jan 9, 2024
kennylam
added
the
Ready to merge
kodiakhq
bot
removed
the
Ready to merge
|
This PR currently has a merge conflict. Please resolve this and then re-add the |
Resolves carbon-design-system#11210 **Changed** - Updates masthead documentation to match expected behavior in v2. - Fixes incorrect React wrapper Masthead demonstrations in Storybook. **Removed** - Removes docs specific to IBM Cloud that have been made unnecessary in v2. <!-- React and Web Component deploy previews are enabled by default. --> <!-- To enable additional available deploy previews, apply the following --> <!-- labels for the corresponding package: --> <!-- *** "test: e2e": Codesandbox examples and e2e integration tests --> <!-- *** "package: services": Services --> <!-- *** "package: utilities": Utilities --> <!-- *** "RTL": React / Web Components (RTL) --> <!-- *** "feature flag": React / Web Components (experimental) -->
jkaeser
added
Ready to merge
kodiakhq
bot
merged commit 67cc49b
into
carbon-design-system:main
12 of 20 checks passed
kodiakhq bot
pushed a commit
that referenced
this pull request
Feb 1, 2024
### Related Ticket(s) Previously fixed in #10986, but that was unwittingly wiped away by #11252 ### Description Fixes an issue where `<c4d-masthead-container>` wipes away user-set L0 data with translations fetched from an API. This PR takes a different approach from the original fix. The root problem is that the masthead container isn't aware of any `l0Data` passed in before setting that property with translation data. I figured we can make the container a little smarter and handle the logic internally rather than adding another property to the masthead composite and complicating the public API. In order to facilitate this, I needed the `ConnectMixin` to pass a reference to the class instance into the `mapStateToProps` function it expects classes to define. This empowers the container to first check if it has L0 data before resorting to the translations API data. As a result, users can reliably set the `l0Data` property on either `<c4d-masthead-container>` or `<c4d-masthead-composite>` and expect the same outcome. ### Changelog **New** - `ConnectMixin` passes a class instance reference as an argument in the `mapStateToProps` function. **Changed** - `<c4d-masthead-container>` will prefer using L0 data passed by user before resorting to automatically fetched translation data. <!-- React and Web Component deploy previews are enabled by default. --> <!-- To enable additional available deploy previews, apply the following --> <!-- labels for the corresponding package: --> <!-- *** "test: e2e": Codesandbox examples and e2e integration tests --> <!-- *** "package: services": Services --> <!-- *** "package: utilities": Utilities --> <!-- *** "RTL": React / Web Components (RTL) --> <!-- *** "feature flag": React / Web Components (experimental) -->
kennylam
pushed a commit
to kennylam/carbon-for-ibm-dotcom
that referenced
this pull request
Mar 21, 2024
…ctor (carbon-design-system#11252) Resolves carbon-design-system#11251 This PR mostly refactors masthead source code to be more maintainable and removes dead code. Other than the style fixes, it should result in no visible changes to the masthead. - Makes necessary functions available to Sass stylesheets to fix style regressions. - Removes Cloud-specific masthead styles. --- - Deprecates `MastheadProfileContent` type. - Deprecates `Translation.mastheadNav` in favor of `Translation.masthead.nav`. - Deprecates `Translation.profileMenu` in favor of `Translation.masthead.profileMenu`. - Removes references to v1 data structures that are no longer in use. --- - Deprecates the `navLinks` Masthead Composite property in favor of the more descriptive `l0Data` property. - Splits out the Masthead Composite's main `render` method into more digestible render methods per logical section. - Untangles the knot that was the `renderNavItems` method by providing dedicated `renderTopNav` and `renderLeftNav` methods for the desktop and mobile experiences. - Extracts distinct chunks of logic into their own methods for readability and in some cases reuse across the newly split render methods. - Removes Cloud-specific masthead. - Removes non-functional `customNavLinks` property. Use `l0Data` or `l1Data` instead. - Removes masthead v1's leftover CTA functionality.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related Ticket(s)
Resolves #11251
Description
This PR mostly refactors masthead source code to be more maintainable and removes dead code. Other than the style fixes, it should result in no visible changes to the masthead.
Changelog
Styles
Changed
Removed
Services Store
Changed
MastheadProfileContenttype.Translation.mastheadNavin favor ofTranslation.masthead.nav.Translation.profileMenuin favor ofTranslation.masthead.profileMenu.Removed
IBM.com Web Components
Changed
navLinksMasthead Composite property in favor of the more descriptivel0Dataproperty.rendermethod into more digestible render methods per logical section.renderNavItemsmethod by providing dedicatedrenderTopNavandrenderLeftNavmethods for the desktop and mobile experiences.Removed
customNavLinksproperty. Usel0Dataorl1Datainstead.