Add Navigation component

[huwshimi]

Done

• Add a component to handle the Vanilla navigation pattern (originally built in MAAS here: feat(ui): build Navigation component. maas-ui#3833).

QA

Storybook

To see rendered examples of all react-components, run:

yarn start

QA in your project

from react-components run:

yarn build
npm pack

Install the resulting tarball in your project with:

yarn add <path-to-tarball>

QA steps

• Go to the navigation page in Storybook and check that the docs make sense and the examples work as expected.

Fixes

Fixes: #134.

[webteam-app]

Demo starting at https://react-components-767.demos.haus

[bartaz]

@huwshimi great, thanks.

Just wondering - we haven't seem to need this component in React in any project, MAAS is the only one I guess. And now it seems that there is work planned on moving to new layout for MAAS, so even MAAS won't need this navigation in React, right?

So, I'm just wondering if it's worth migrating it and maintaining here in React components?

[huwshimi]

I think it'd be useful to have in react-components for any project adopting React. It's been quite painful to maintain the header in MAAS over the years because we were never quite aligned with Vanilla (or didn't keep up with the latest Vanilla markup).

I'd say any new project would find it quite a barrier to have to implement this themselves and sounds like there are other projects that could use this: #134 (comment).

I do understand that this will be less useful to MAAS within the next year or so when we adopt the new layout, but that could be quite a while off still.

[bartaz]

I think it'd be useful to have in react-components for any project adopting React. It's been quite painful to maintain the header in MAAS over the years because we were never quite aligned with Vanilla (or didn't keep up with the latest Vanilla markup).

@huwshimi Sure, thanks for more context. I have it in my review queue.

[petermakowski]

I opened a PR with some changes to this component in maas-ui: canonical/maas-ui#3908. If that looks ok to you, would be good to include them here before merge @huwshimi

[petermakowski]

@huwshimi I noticed that the mobile nav background doesn't match that of vanilla.

[huwshimi]

@huwshimi I noticed that the mobile nav background doesn't match that of vanilla.

It turns out this is an inconsistency in Vanilla. I've filed an issue: canonical/vanilla-framework#4461.

[huwshimi]

I opened a PR with some changes to this component in maas-ui: canonical-web-and-design/maas-ui#3908. If that looks ok to you, would be good to include them here before merge @huwshimi

Thanks, I've updated this PR with those changes.

[petermakowski]

Not sure if this was like that last time I reviewed, but dropdown items on mobile look broken (do not extend to full width).

This implementation

https://react-components-767.demos.haus/?path=/story/navigation--dropdown

Vanilla example

https://vanillaframework.io/docs/patterns/navigation#dropdown

[petermakowski]

@huwshimi I noticed that the mobile nav background doesn't match that of vanilla.

It turns out this is an inconsistency in Vanilla. I've filed an issue: canonical-web-and-design/vanilla-framework#4461.

I couldn't find a single mention of the .has-menu-open class in vanilla documentation. 🤔 Might we be using something that's deprecated here?

[bartaz]

I couldn't find a single mention of the .has-menu-open class in vanilla documentation. 🤔 Might we be using something that's deprecated here?

It was recently introduced with updates to new logo branding and other navigation changes. This class is added by JS when a menu is opened on mobile: https://vanillaframework.io/docs/examples/patterns/navigation/search-dark

It's possible that not all examples have it. It's only required when you have search in the navigation.

But I guess we should have made this more clear in the docs. Or at least provide class reference for the top nav component.

[huwshimi]

@huwshimi I noticed that the mobile nav background doesn't match that of vanilla.

It turns out this is an inconsistency in Vanilla. I've filed an issue: canonical-web-and-design/vanilla-framework#4461.

I couldn't find a single mention of the .has-menu-open class in vanilla documentation. 🤔 Might we be using something that's deprecated here?

If you scroll down to the Expanding search box example you can see the grey background: https://vanillaframework.io/docs/patterns/navigation#expanding-search-box

Edit: you need to change the toggle to JS to see how "has-menu-open" is used.

[huwshimi]

Not sure if this was like that last time I reviewed, but dropdown items on mobile look broken (do not extend to full width).

That should be fixed with the next Vanilla release: canonical/vanilla-framework#4444.

Not sure if that means we should hold off landing this until it's not broken.

[bartaz]

Not sure if this was like that last time I reviewed, but dropdown items on mobile look broken (do not extend to full width).

That should be fixed with the next Vanilla release: canonical-web-and-design/vanilla-framework#4444.

Not sure if that means we should hold off landing this until it's not broken.

It's practically ready to release, I'll try to publish it today.

[petermakowski]

Not sure if this was like that last time I reviewed, but dropdown items on mobile look broken (do not extend to full width).

That should be fixed with the next Vanilla release: canonical-web-and-design/vanilla-framework#4444.

Not sure if that means we should hold off landing this until it's not broken.

The new vanilla has been released and merged to main and the issue with the buttons width I mentioned is fixed after updating this branch with main. This should be good to merge now.