feat(@clayui/navigation-bar): Add sr-only description for mobile view

[SelenaAungst]

https://liferay.atlassian.net/browse/LPD-29692

[pat270]

Instead of sr-only, I think it's better if we use aria-label on the button instead. I think we need to be able to pass in a custom message to the aria-label for translation purposes.

[SelenaAungst]

Ah I see, I will make that change

[SelenaAungst]

@pat270 I made the change but for some reason I was not able to run yarn develop to test it, are you able to test this one? I am not sure why the CI/test check is failing as well, it looks unrelated to my changes

[matuzalemsteles]

@SelenaAungst you need to recreate the test snapshots by running yarn test -u. Since you updated the HTML this is reflected in the snapshot tests which are a way to ensure that the DOM remains the same. We only update the snapshot when the DOM change is intentional like your use case, just to avoid regressions.

In relation to this we should move this to be configured via props in the component, something like:

<ClayNavigationBar
  messages={{
    close: 'Close',
    open: 'Open',
    trigger: '{0} Menu, Current Page: {1}',
  }}
>
 {...}
</ClayNavigationBar>

So internally you can use these properties to form the aria-label of the trigger.

<ClayButton
  aria-expanded={expanded}
  aria-label={sub(
    messages.trigger,
    [expanded ? messages.close : messages.open, triggerLabel]
  )}

[SelenaAungst]

Ah got it, thanks @matuzalemsteles! So do the ClayNavigationBar props still go in index.tsx or somewhere else? I will make the change to aria-label and run yarn test -u after this is complete.

[matuzalemsteles]

So do the ClayNavigationBar props still go in index.tsx or somewhere else?

Yes, you just need to define this new API type in the component props as well and add the type.

[SelenaAungst]

Thanks @matuzalemsteles! I tried to add these changes and update tests. I am not positive this is correct so please let me know if there are any additional changes I can make.

[matuzalemsteles]

@SelenaAungst looks good, just adding a comment to avoid a possible breaking change issue I forgot to mention.

[matuzalemsteles]

Just to avoid the problem of breaking changes in components, we can add a default value here. This way, whoever is using it will not see the application build breaking.

Suggested change

	messages,
	messages = {
	close: 'Close',
	open: 'Open',
	trigger: '{0} Menu, Current Page: {1}',
	}

[matuzalemsteles]

Here we also need to make this API optional to avoid the same problem I mentioned above.

[SelenaAungst]

I added the default values, let me know if it looks good @matuzalemsteles

[matuzalemsteles]

LGTM! I think the build that is failing is not related to your changes.