-
Notifications
You must be signed in to change notification settings - Fork 6
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
[#1535] Added subheading anchors to product page sidebar #724
Conversation
c7429bb
to
8f2371e
Compare
Codecov Report
@@ Coverage Diff @@
## develop #724 +/- ##
===========================================
+ Coverage 96.27% 96.31% +0.04%
===========================================
Files 674 684 +10
Lines 24007 24217 +210
===========================================
+ Hits 23113 23325 +212
+ Misses 894 892 -2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
53d1ca1
to
7cd8019
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.
Two changes are needed:
- Make it so only the Headings that are scrolled into will display as Bold, so not the entire list-item
- Bring back the correct blue/scroll-color (link) line that appears and shows the progress of the scroll (see image)
If you need more: You can add more scroll-classes by adding more active/open class rules for the Gumshoe JS here: src/open_inwoner/js/components/anchor-menu/anchor-menu.js
Example of primary-color 'voortgangsbalk/progress-line' on the left of each link/heading that is scrolled onto:
More info about Gumshoe:
https://github.com/cferdinandi/gumshoe
src/open_inwoner/components/templates/components/AnchorMenu/AnchorMenu.html
Outdated
Show resolved
Hide resolved
@@ -82,6 +82,30 @@ | |||
} | |||
} | |||
|
|||
// nested list | |||
&__sublist { | |||
list-style-type: '- '; |
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.
I would like to show this to a designer actually - because I think this should be 'list-style-type:none; so it looks cleaner and ore in line with the rest of the website, but i'm really not sure about this one. So perhaps leave it as is.
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.
After playing around a bit with different styles, I think you're right. Changed it accordingly.
@Bartvaderkin Why do you keep changing the titles of my PRs? I use the imperative mood for a reason. See here: https://git.kernel.org/pub/scm/git/git.git/tree/Documentation/SubmittingPatches?h=v2.36.1#n181 |
d93781e
to
67ca2cf
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.
Nice work - just one more detail to improve: on mobile there should not be a left-margin in the dropdown for the H1 level; adding a bit of a margin to the H2 level is OK for now (look at https://open-inwoner-test.maykin.nl/onderwerpen/zorg-en-ondersteuning/producten/logeeropvang/ if you need an example):
- color only focused part of border - bold-face only focused heading/sub-heading - create namespace for subheading slugs to avoid conflicts
67ca2cf
to
faac2ca
Compare
@jiromaykin I replaced the magic numbers with variables. The spacing to the left of the |
We're using capitalized Verbs in this project. Describe what you did, not the task: "Added this", "Updated that". Let's stick with the patterns and not mix and mess. |
Taiga: #1535