Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates to mojMenu (Fix #62) #68

Merged
merged 5 commits into from
Aug 19, 2019
Merged

Updates to mojMenu (Fix #62) #68

merged 5 commits into from
Aug 19, 2019

Conversation

simonwhatley
Copy link
Contributor

Fix #62

  • Rename mojMenu to mojButtonGroup
  • Add support for all attributes of govukButton
  • Add support for govuk-button--warning type
  • Add documentation to component readme
  • Add greater number of examples to the frontend app

@simonwhatley simonwhatley self-assigned this Aug 14, 2019
@simonwhatley simonwhatley temporarily deployed to moj-frontend-staging-pr-68 August 14, 2019 11:22 Inactive
Copy link
Contributor

@adamsilver adamsilver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know we've talked offline but in the spirit of being open I'll summarise my thoughts here:

  1. https://inclusive-components.design/menus-menu-buttons/ is worth referencing and covers all sorts of menus and the difference between ‘true menus’

  2. I don't think it's right to name the JS component button-group.js because the JS being activated turns it very specifically into a toggle menu—as in it's no longer just a button group.

  3. I think the JS behaviour should be changed in accordance with (1). Specifically that it's not a true menu, it's a bunch of links (that happen to look like buttons)—meaning that focus shouldn't move to the first item in the menu when opened. This happens to drastically simplify the script.

  4. With (3) said, it could be more ‘true menu’ like if it was used in a multi-select form with multiple actions. The items would be submit buttons (and not links/navigation). But the main use case is (3) and I don't have any evidence that the list being made of submit buttons requires a change of behaviour anyway.

  5. All of that to say that I think my favourite name for this is navigation-menu.js, at least for the JS part.

Change from button group to button menu
@simonwhatley simonwhatley requested a deployment to moj-frontend-staging-pr-68 August 19, 2019 13:44 Abandoned
@adamsilver adamsilver merged commit 42c2576 into master Aug 19, 2019
@simonwhatley simonwhatley deleted the fix-62 branch August 19, 2019 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update mojMenu
2 participants