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.

Sign up for GitHub

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

Jump to bottom

Tabs #750

Closed
wants to merge 37 commits into from
Closed

Tabs #750

wants to merge 37 commits into from

Conversation

andymantell
Copy link
Contributor

@andymantell andymantell commented Jun 25, 2021 •
edited
Loading

Description

Porting the tabs component from govuk-frontend.
See nhsuk/nhsuk-service-manual-community-backlog#279

Checklist

@andymantell andymantell added the work in progress Work in progress, not ready for review label Jun 25, 2021
@andymantell
Copy link
Contributor Author

@tomdoughty @chrimesdev Couple of things I'd like to discuss with you before I go any further...

  1. govuk-frontend uses data-module attributes to attach JavaScript behaviours to but this doesn't seem to be a thing in nhsuk-frontend. Would you prefer that the JS is bound to the tabs class which is used for the styling? This would mean people wouldn't be able to do "serverside tabs" with a full page load on each tab, since the JS would get in the way. Or would you prefer the JS and the tab-like presentation be separable as in the govuk-frontend case?

  2. I've pretty much just brought the govuk-frontend tabs JS in as it stands with a couple of minor tweaks to make it "fit in" with the way NHS components are initialised. I don't know whether you want me to do any further refactoring to make it fit some notion of a "house style" however? The JS doesn't look like the rest of our JS (use of prototype etc etc). The upside of this of course is that if govuk make any fixes at their end then they're fairly easy to bring in. (I'm aware the linting rules don't currently pass - I will address that)

@benturner1976 benturner1976 mentioned this pull request Oct 25, 2021
Open
@chrimesdev chrimesdev mentioned this pull request Nov 4, 2021
Merged
6 tasks
@andymantell andymantell marked this pull request as ready for review June 13, 2022 12:25
@DomBaker DomBaker removed the work in progress Work in progress, not ready for review label Jun 14, 2022
@wa-rren-dev
Copy link
Contributor

Closing here as tabs in this form are now merged into alpha v7

@wa-rren-dev wa-rren-dev closed this Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andymantell @wa-rren-dev @DomBaker