Add icon to header to link to User Guide #7821
Conversation
|
neat - any chance of making the icon a little bit bigger? 20% or so - that's my $.02, which could get vetoed - looks good though all in all |
|
@dartajax No problem on the size change. I also noticed I need to adjust it for smaller screens, so I'll fix that, too. |
| assert.dom('.user-guide-link svg.fa-circle-question').exists(); | ||
| assert.dom('.user-guide-link svg.fa-circle-question title').hasText('Ilios User Guide'); | ||
|
|
||
| this.owner.lookup('service:intl').setLocale('es'); |
There was a problem hiding this comment.
You don't need to test the translations, but if you want to follow the pattern in https://ember-intl.github.io/ember-intl/docs/guide/testing to set the locale or test the untranslated output.
There was a problem hiding this comment.
Updated tests to be more in line with convention.
|
Some alternatives for icons:
@dartajax What say you? |
|
Might be the "regular" I don't love. We have a license for everything. |
|
Oddly, though, the Regardless, I ended up using the |
| @use "../mixins" as m; | ||
|
|
||
| .user-guide-link { | ||
| @include m.header-menu; |
There was a problem hiding this comment.
I don't think this is quite a header menu, too many styles in there don't apply here.
There was a problem hiding this comment.
I removed the mixin and used a simpler style to get the same effect.
|
|
||
| @include cm.for-phone-only { | ||
| /* stylelint-disable property-disallowed-list */ | ||
| font-size: 3.5vw; |
There was a problem hiding this comment.
I don't love using this disallowed property, Could this be done by controlling width and height of the icon?
There was a problem hiding this comment.
I removed this disallowed property and used height/width instead.
| align-items: center; | ||
| color: c.$white; | ||
| display: flex; | ||
| height: 1.85em; |
There was a problem hiding this comment.
I'm wondering if this can be done as a percentage of the container. Not sure, just feels a little arbitrary and might break if we accidentally slightly change the header height.
There was a problem hiding this comment.
I changed this to use pixels instead. Using percentage of the container did not seem to work.
|
Code looks good, assigning to @dartajax for visual review and merging. |
…ight on top of it
…e like usual translation convention
michaelchadwick
force-pushed
the
frontend-3759-add-guide-icon-to-header
branch
from
2fb40ba
to
2e3a0b0
Compare
|
Percy just needs approval of the changes @dartajax, because this header is on every page it's a lot of changes, but still worth checking through I think. |
Fixes ilios/ilios#3759.
Created new
UserGuideLinkcomponent that displays an icon to click that opens the Ilios User Guide in a new window. It has translations for the<svg><title>element, and a simple component test.