search

mozilla / bedrock

Making mozilla.org awesome, one pebble at a time
https://www.mozilla.org
Mozilla Public License 2.0
1.16k stars 913 forks source link

Navigation JS fixes not picked up from upstream Protocol #14770

Open alexgibson opened 1 week ago

alexgibson commented 1 week ago

Description

The main nav in bedrock is missing fixes such as https://github.com/mozilla/protocol/issues/847 because it uses it's own (now outdated) copy of the nav JS.

I'm not sure what the best fix is. Maybe it's something we can address in the site refresh?

craigcook commented 1 week ago

hrm yeah, bedrock's nav is a customized component that never got backported to Protocol so all the names are different. We could implement the same fixes in bedrock's custom JS easily enough.

alexgibson commented 1 week ago

I was wondering if we could actually revert to using Protocol's JS instead? It seems to be mostly duplicated just to change some selector names, and it seems like potentially a lot of effort to duplicate all the fixes?

craigcook commented 1 week ago

Yeah, it was literally just to avoid naming conflicts. The redesigned nav restyled a bunch of things but the underlying structure is mostly the same, so I ended up dropping the mzp- prefix to avoid issues with core Protocol. Then had to dupe the script just to work with the different names. If we use Protocol's JS we'd need to add the prefix to the nav and try to avoid any styling conflicts as well, but that might not be too hard. Worth trying anyway.

alexgibson commented 1 week ago

Possible resolutions:

  1. We update the JS in bedrock with the bug fixes that have landed in Protocol since the navigation was forked.
  2. We update the navigation class names / selectors in bedrock to match what the Protocol JS expects, and stop relying on the now outdated JS in bedrock.
  3. We finish back porting the navigation CSS in bedrock to Protocol, then publish an NPM update and bring everything back in.
  4. We fix this when the site refresh happens (it's still unclear if we'll use the existing navigation JS or start a-fresh?)