-
Notifications
You must be signed in to change notification settings - Fork 50
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
New layout #1204
New layout #1204
Conversation
✅ Deploy Preview for laughing-payne-b9fbd2 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
694022a
to
a6c6b50
Compare
18b9e57
to
8d7dd8a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a big improvement, the navigation is much more readable!
@@ -13,9 +13,9 @@ | |||
{% endif %} | |||
{% endif %} | |||
|
|||
<footer class="contribute" data-proofer-ignore> | |||
<a href="/admin/#/collections/{{ collection_name }}/entries/{{ entry_name }}">Edit this page</a> | |||
<div class="contribute" data-proofer-ignore> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the change from <footer>
to <div>
a semantic change or just a styling one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Semantic - the large screen layout is now horizontal, so I'm considering the left side the header
and right main
. On small screens the contribute section isn't shown. Not sure whether this is good practice though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On a stylistic note, it would actually work fine if it was still a footer given the fixed positioning. I'm going back and forth on whether it should live in the header, main, or footer semantically 🤔
Constrain the nav and contribute element to the same width as the related content menu, allowing the main article to stretch the full height of the viewport
- Reduce the font weight - Add the dxw 'D' as a list item indicator (excluding the back link), which is a style adopted from our main site's [impact report][1] - Remove the second line indent [1]: https://www.dxw.com/dxw-impact
Index pages (i.e. pages which have children but not their own content) include the same partial as the nav menu. This menu includes a 'Contact dxw' link, which looks a little out of place in the body of the index page. This makes it so that the 'Contact dxw' link will only appear in the nav, not the list of child pages on index pages
Closes #1199
Closes #1142
See those issues for more context
Summary of changes
On large screens:
On all screen sizes:
Previews
Interactive
Available at https://deploy-preview-1204--laughing-payne-b9fbd2.netlify.app
Video
Screen.Recording.2023-10-16.at.21.08.30.mov