fix(masthead): container should respect user's L0 data #11420
Conversation
It can be useful to check instance data before reaching out to Redux store to set properties.
jkaeser
added
package: web components
|
Deploy preview created for package Built with commit: 1b5b8ff5e1dbda706e00c2288a8cf3310409a618 |
|
Deploy preview created for package Built with commit: 1b5b8ff5e1dbda706e00c2288a8cf3310409a618 |
|
Deploy preview created for package Built with commit: 1b5b8ff5e1dbda706e00c2288a8cf3310409a618 |
|
Deploy preview created for package Built with commit: 1b5b8ff5e1dbda706e00c2288a8cf3310409a618 |
jkaeser
requested review from
ariellalgilmore,
annawen1,
m4olivei and
andy-blum
and removed request for
a team
|
@annawen1 I'm curious to hear your thoughts on this one, since it takes a different approach to a fix you added back in September. |
Oh yeah, I didn't think to look into the |
IgnacioBecerra
added
the
Ready to merge
kodiakhq
bot
merged commit cf2f60d
into
carbon-design-system:main
### Related Ticket(s) none ### Description #11420 ensures support for customizing the L0 nav items, but it doesn't account for the `navLinks` property. This PR adds that support, and it also includes a test to make sure L0 customization is working as expected. ## To test 1. Visit this Codepen example: https://codepen.io/jakaeser/pen/LYavrKK 2. See that the custom L0 nav items are not applying. 3. Comment out the v2 latest CDN <script>, then uncomment the deploy preview CDN <script> 4. Verify the custom L0 nav items apply. 5. In the Pen's JS, replace the `l0Data` property with `navLinks`. Verify the custom L0 items still apply. ### Changelog **New** - Adds an e2e test for customizing L0 items. **Changed** - Supports `navLinks` property when using `<c4d-masthead-container>` and customizing L0 items. - Corrects the prefixes used in masthead e2e tests. <!-- 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) -->
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
l0Datapassed 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
ConnectMixinto pass a reference to the class instance into themapStateToPropsfunction 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
l0Dataproperty on either<c4d-masthead-container>or<c4d-masthead-composite>and expect the same outcome.Changelog
New
ConnectMixinpasses a class instance reference as an argument in themapStateToPropsfunction.Changed
<c4d-masthead-container>will prefer using L0 data passed by user before resorting to automatically fetched translation data.