Problem/Motivation
Single directory components (SDC) is a new way to theme Drupal. Instead of scattering related files around your theme, they're contained to one directory. The primary issue for SDC is at #3313520: Single directory components in core.
The Demo Umami team has decided that their theme can depend on an experimental module like SDC. This allows us to have the work in this ticket merged even when #3352256: [META] Move code from the experimental SDC module to core is not committed.
Header component
As part of SDC's roadmap (see #3345922: Single Directory Components module roadmap: the path to beta and stable), we want to convert Umami components to use SDC. For this task I'm choosing the Header component, which includes markup, CSS, and JS.
This component is a good fit because:
This first component in core which explains how to use javascript in component and define dependency.
This menu was defined without dependencies. But i think we need to show how to use them in SDC
Testing instructions
- Install Umami
- Switch to mobile
- Test burger menu
- Switch to desktop
- Test main menu for regressions
Comment | File | Size | Author |
---|---|---|---|
#10 | 3404302-nr-bot.txt | 3.8 KB | needs-review-queue-bot |
burger.gif | 1.28 MB | finnsky |
Issue fork drupal-3404302
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
finnsky CreditAttribution: finnsky at Skilld commentedComment #4
finnsky CreditAttribution: finnsky at Skilld commentedComment #5
finnsky CreditAttribution: finnsky at Skilld commentedComment #6
smustgrave CreditAttribution: smustgrave at Mobomo commentedApplied the MR locally taking before screenshots (won't upload as the video is there) and the after screenshots which look exactly the same.
Looking this seems to be the first example using css, js, and images so awesome!
Comment #7
lauriiiPosted feedback in the MR.
Comment #8
finnsky CreditAttribution: finnsky at Skilld commentedThank for review. Gonna quickfix
Comment #9
finnsky CreditAttribution: finnsky at Skilld commentedComment #10
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #11
finnsky CreditAttribution: finnsky at Skilld commentedComment #12
finnsky CreditAttribution: finnsky at Skilld commentedI think Header component will be cleaner to understand.
Please review!
Comment #13
markconroy CreditAttribution: markconroy as a volunteer and at Annertech commentedThis looks great. Thanks @finnsky.
RTBC from my point of view.
Comment #14
quietone CreditAttribution: quietone at PreviousNext commentedI'm triaging RTBC issues. I read the IS and the comments. I didn't find any unanswered questions or other work to do.
Leaving at RTBC.
Comment #15
quietone CreditAttribution: quietone at PreviousNext commentedI also meant to adjust capitalization in the title and remove the '.'.
Comment #17
catchComment #21
nod_Committed bc4b787 and pushed to 11.x. Thanks!