Bootstrap tooltip and glossary UI fixes#334
Conversation
…fy ttl parsing, add opt-in button for highlighting
jmillanacosta
changed the title
|
You can remove the custom bootstrap css to make it behave like a common tooltip, but I figured that could bring some unexpected issues, so if we decide to have it behave like a normal tooltip (show it below the word) I'd still keep the custom class |
| 'script', 'style', 'noscript', 'iframe', 'object', | ||
| 'button', 'select', 'option', 'textarea', 'input', | ||
| 'a', 'label', 'code', 'pre', 'kbd', 'samp', 'var', | ||
| 'nav', 'footer', 'header', |
There was a problem hiding this comment.
I would also add h1, h2, ..., h6 here. Glossary in headlines looks funny. Often the text below picks up the title again. If not we should write more explanatory text anyways. But since now we have it user requested I'm less strict on this. The user clearly wants explanation.
Example VHP4 safety headline:
There was a problem hiding this comment.
Lovely! 👍 The less we have here the less can interfere.
There was a problem hiding this comment.
bootstrap-custom.css is compiled via npx, do not change manually.
If will not be persisted if people modify the sass maps and compile how it is designed. Guidelines are in the readme on how to go from sass maps to bootstrap-custom.css.
There was a problem hiding this comment.
Also this would be the place where we would systematically change the appearance of all tooltips. The black is a bit very sad in contrast to our vhp4-safety colorscheme. But I would make that a separate issue. No need to act on this now.
There was a problem hiding this comment.
I assumed that was stale because of the many lines that disappeared when compiling, fixed it
static/js/glossary_highlighter.js
Outdated
| function setGlossaryVisible(visible) { | ||
| glossaryActive = visible; | ||
| document.querySelectorAll('[data-vhp-glossary]').forEach(span => { | ||
| span.classList.toggle('bg-vhpblue', visible); |
There was a problem hiding this comment.
The accent color is different on each page, however I would like to argue in favour of bg-vhplight-blue:
- it is the accent color used in most of the website. And due to the strip at the bottom being vhplight-blue and the shape being also a stripe it would be most harmonic in vhplight-blue
static/js/glossary_highlighter.js
Outdated
| glossaryActive = visible; | ||
| document.querySelectorAll('[data-vhp-glossary]').forEach(span => { | ||
| span.classList.toggle('bg-vhpblue', visible); | ||
| span.classList.toggle('bg-opacity-10', visible); |
There was a problem hiding this comment.
Now the user actively WANTS to see the glossary, I don't think we still need to be as subtle as we were in the general one-for-all solution.
Hence, I propose to turn it up a notch:
bg-opacity-50
static/js/glossary_highlighter.js
Outdated
| glossaryActive = visible; | ||
| document.querySelectorAll('[data-vhp-glossary]').forEach(span => { | ||
| span.classList.toggle('bg-vhpblue', visible); | ||
| span.classList.toggle('bg-opacity-10', visible); |
There was a problem hiding this comment.
I also propose to add:
rounded
Even prettier, I think would be rounded-pill px-2 but I can live with rounded only, albeit it mimics a lot our buttons (which not being too bad actually given that we want people to click on the glossary buttons now).
templates/base.html
Outdated
| <!-- Tools Button Group --> | ||
|
|
||
| <!-- Glossary Toggle --> | ||
| <button id="glossary-toggle" class="btn btn-outline-vhpblue btn-sm text-nowrap" type="button" |
There was a problem hiding this comment.
The button radio-toggle is not really a link, so I would like to make visual distinction between buttons and the toggle. Bootstrap5 comes with a fromcheck switch:
Hence change this buttom to something along these lines:
<div class="form-check form-switch">
<input class="form-check-input" type="checkbox" role="switch" id="switchCheckChecked" checked>
<label class="form-check-label" for="switchCheckChecked">Checked switch checkbox input</label>
</div>
templates/base.html
Outdated
| <!-- Data button --> | ||
| <button class="btn btn-vhpteal mt-2" onclick="location.href='/data'" data-bs-dismiss="offcanvas">Data</button> | ||
|
|
||
| <button class="btn btn-outline-vhpblue mt-2" type="button" data-vhp-glossary-skip |
static/js/glossary_highlighter.js
Outdated
| const btn = document.getElementById('glossary-toggle'); | ||
| if (btn) { | ||
| new bootstrap.Tooltip(btn); | ||
| btn.addEventListener('click', () => { |
There was a problem hiding this comment.
The current solution depends on the user making the choice to glossary on each subpage again and again.
I suggest we either store sessionStorage (per tab and clears after user closes), or localStorage (persistence across browser closing) to save the user's choice. Then roll with that, will be a smoother experience.
There was a problem hiding this comment.
uses localStorage and finds the value on boot
This breaks |
That's why I didn't build scss at first, better to patch it in main before merging this branch |
Can we add to the sass map that we style the switch in line with other colors? I suggest the color of the search button? $form-check-input-checked-bg-color: map-get($theme-colors, "vhppurple");
$form-check-input-checked-border-color: map-get($theme-colors, "vhppurple");Also wouldn't mind if the tooltip bg color were the 
|
👍 |
|
Depends on #336 |
Actually, #341 changes the search menu, so has no longer a search button which was basically without function, so a bit more room for coloring of the switch toggle, I would just not make it yet another color (and bg-primary it is atm). |
3 participants
Screencast.from.11-03-26.14.22.04.webm
The button can be moved, and since it's clicking now maybe it makes mroe sense to have a true tooltip and not force it to the bottom left corner, what would look better?