UHF-9866: Arrow and underline refactoring #1101
Conversation
…oter, change popular services paragraph to use display inline-block styling
…l occasions. 2px when hovered and 1px when not
|
|
|
|
|
Looks good! One thing from etusivu instance:
For all the instances:
|
There was a problem hiding this comment.
I checked the code first to give you time to check it out while I'm clikcing the functional stuff.
The code looks clean, but I was as usual, a bit nit picky on some details. You can check what you think about these things and we can discuss further. Good stuff nonetheless, it's great to see progress on this field after such a long time of discussions. Good job!
I'll get back about the rest of the PR after I've clicked it through.
|
|
||
| // Make the clickable area conform with WCAG AAA standard and adjust the text slightly higher than | ||
| // the center to make it look balanced. | ||
| padding-bottom: calc($-item-link-padding * 0.6667); |
There was a problem hiding this comment.
Explain these magic numbers. Preferrably calculate them using the $wcag-touch-target-size-aa or $wcag-touch-target-size-aaa
There was a problem hiding this comment.
Added better explanation :)
|
|
||
| @include breakpoint($breakpoint-s) { | ||
| display: block; | ||
| min-height: calc(get-line-height('h4.breakpoint_l', $unremify: true) + $spacing-quadruple); |
There was a problem hiding this comment.
- Why use calc here instead of calculating it in scss?
- Can you clarify why you're adding $spacing-quadruple to line-height?
There was a problem hiding this comment.
I adjusted these min-heights quite a bit and commented them more clearly so can you check them again 🙏
| text-decoration: none; | ||
| } | ||
| // Make the clickable area conform with WCAG AAA standard. | ||
| padding-block: calc($-item-link-padding * 0.5); |
There was a problem hiding this comment.
Why are you using calc to calculate something in browser that scss could calculate already?
There was a problem hiding this comment.
Moved the calcuation to the top and removed the calc-function.
|
|
||
| // The padding below it are used to make the link correct minimum height for the WCAG AAA standard. | ||
| padding-block: $-service-item-padding; | ||
| padding-left: $spacing-quarter; |
There was a problem hiding this comment.
You could use padding-inline-start to handle the dir=rtl situation automatically without the need for padding-left: 0 later.
There was a problem hiding this comment.
I changed this now but I think we actually have an issue with this. It seems to be that our compiler converts padding-inline-start to padding-left and that shouldn't happen because the whole thing stops working if it does that. I checked the compiled css and for me it does that at least. Lets investigate this!
templates/field/field--paragraph--field-service-links--popular-service-item.html.twig
Outdated
Show resolved
Hide resolved
…s is written on a separate row and using remify function
|
@annadruid No bullet points for list of links with images 😄 just my copy-paste-mistake.
I am not quite sure but I think it should be at least logical. I will change it so that everywhere where the link is underlined the focus gets the thicker underline as well. |
|
|
|
|
|
|
UHF-9866
What was done
How to install
Other instances than Etusivu, KYMP or KASKO
git pull origin devmake freshcomposer require drupal/hdbt:dev-UHF-9866make drush-crEtusivu, KYMP and KASKO
git fetch && git checkout UHF-9866make freshcomposer require drupal/hdbt:dev-UHF-9866make drush-crHow to test
It might be easiest to start with Etusivu instance since most of the components are used there on the front page and then move to KYMP and KASKO that have few special cases. There is a link after each component to find one example of them. You can also create a test page with the components if that is easier to test for you.
Continuous documentation
Other PRs