Dc subnav add editions switch #6012
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: -572 kB (-25%) 🎉 Total Size: 1.68 MB
ℹ️ View Unchanged
|
oliverlloyd
reviewed
Sep 19, 2022
There was a problem hiding this comment.
This is looking pretty great. I've got some thoughts and comments on some small areas but nothing major.
Also, @HarryFischer are there any design requirements for how this edition dropdown should look on mobile?
Oh, and one more very small thing: would it be possible to update the before and after images to be DCR before and after. I think that before image is of Frontend. We could also include a screenshot of frontend and have three columns?
dotcom-rendering/src/web/components/Nav/ExpandedMenu/Column.tsx
Outdated
Show resolved
Hide resolved
| ))} | ||
|
|
||
| <li> | ||
| <ThemeProvider theme={{ ...buttonThemeBrand }}> |
There was a problem hiding this comment.
I know that there is an existing use case for using Emotion theming but I'm nervous adding more as Emotion using React context to provide this value and I suspect in this case we don't need such an advanced solution and can style this button using a more basic approach. Happy to pair on looking at this in more detail.
There was a problem hiding this comment.
dotcom-rendering/src/web/components/Nav/ExpandedMenu/Columns.tsx
Outdated
Show resolved
Hide resolved
dotcom-rendering/src/web/components/Nav/ExpandedMenu/Column.tsx
Outdated
Show resolved
Hide resolved
dotcom-rendering/src/web/components/Nav/ExpandedMenu/Columns.tsx
Outdated
Show resolved
Hide resolved
This reverts commit b1b08c4.
…ardian/dotcom-rendering into dc-subnav-add-editions-switch
mxdvl
approved these changes
Sep 28, 2022
| ))} | ||
|
|
||
| <li> | ||
| <ThemeProvider theme={{ ...buttonThemeBrand }}> |
There was a problem hiding this comment.
Closed
Co-Authored-By: Ioanna Kokkini <ioannakok@users.noreply.github.com>
⚡️ Lighthouse report for the changes in this PRLighthouse tested 2 URLs Report for Article
Report for Front
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this change?
This adds the editions switch to DCR subnav
Why?
Parity with frontend
Screenshots