a11y reading options bar#5340
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Limitations / Future Development.
|
64d0182 to
9147cc4
Compare
ScreenshotsNon-article pages
Article pagesAlso include the draw-attention animation toggle, cite, email and print options, depending on settings. And material doesn't have the cite modals. 
Pre-set colours and fontsI've created a doc to describe how to amend the fonts and colours, they are kept in 
TogglesI've copied the Italics toggle from the previous dyslexia mode, and added one for |
2f8f1c2 to
00fce79
Compare
This reverts commit 6cbf468. The change could be better made by targetting all links, and was causing tests to fail.
ajrbyers
left a comment
ajrbyers
left a comment
There was a problem hiding this comment.
I've added a couple of inline comments but generally:
- The bar should remain in its original place as with OLH on all four themes, it feels awkward at the top of the page
|
|
||
| 1. Put the font files in `src/static/common/fonts/<family>/` (`.woff2`/`.woff` | ||
| preferred; `.otf`/`.ttf` work too). Include the font's licence file. | ||
| 2. Add an `@font-face` block to `src/static/common/css/text_readability.css`: |
| # the single source of truth shared with the bar template and the JS. Server-side | ||
| # validation means a stale or tampered stored value can never lodge an | ||
| # unresolvable preference (the same guarantee the JS rollback gives live). | ||
| _HEX_COLOUR_RE = re.compile(r"^#[0-9a-fA-F]{6}$") |
There was a problem hiding this comment.
No need to redefine this here as we can get it from the already imported text_format
| // Keep the menu-button labels in step with the selection. | ||
| setLabelText('.tf-font-select', (FONTS[state.font] || FONTS['default']).label); | ||
| setLabelText('.tf-scheme-select', (COLOURS[state.scheme] || COLOURS['default']).label); | ||
| document.querySelectorAll('.tf-custom-colours').forEach(function (block) { |
| if (document.readyState === 'loading') { | ||
| document.addEventListener('DOMContentLoaded', initialise); | ||
| } else { | ||
| initialise(); |
There was a problem hiding this comment.
Do we need to eager call initialise here if we're calling it on demand below?
| add marker class `.text-format-region` to define the target parent(s). | ||
| {% endcomment %} | ||
| <section aria-label="{% trans 'Reading options' %}" class="invisible-landmark reading-options-landmark"> | ||
| <div id="tf-bar" data-sticky-container class="row reading-options-bar{% if text_format_preferences.hideReadingBar %} tf-bar-hidden{% endif %}"> |
There was a problem hiding this comment.
data-sticky is on two divs here.
@ajrbyers I disagree. I tried placing the bar where it was on OLH originally - but it means this was then confusing as it cannot be universally added to non-article pages and the majority do not have banner images. Placing it in different places on different pages felt clunky and meant different CSS was needed in different places. Placing it at the top of the page is the only 'universal' position I could find that worked across other templates, e.g. news and CMS pages. And then I made it sticky so once you scroll down past the banner image it will appear at top of window anyway. |
Ah right I was thinking about it specifically from the article POV. I'll take another look this morning. |
|
@ajrbyers I've added a commit which sets the default to hide the reading options bar. It is then found in the accessibility menu. I hope that is a good compromise to get this out the door. |
3 participants
closes #5337
openlibhums/clarity#10 ports these changes to clarity.