Narrow regions; Split borders; Pointer events; Full-height/width regions #179
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* This uncouples the label from the border, so it can be repositioned if needed. * We have an extra element to track now, but it gives much greater flexibility. * Remove some magic numbers (outline and border widths). Note: this needs testing on pages that scroll sideways.
* Ensure that the label occupies only one line. * Ensure that the label prefers being on the left. * Ensure that the point at which the label flips between left and right is consistent.
matatk
commented
Jun 4, 2018
src/static/content.focusing.js
Outdated
|
|
||
| // Is part of the label off-screen? | ||
| const labelBounds = label.getBoundingClientRect() | ||
| if (labelBounds.left < 0 || label.scrollWidth > label.offsetWidth) { |
There was a problem hiding this comment.
Is the scrollWidth stuff needed now that the "left" property is removed first?
matatk
commented
Jun 4, 2018
src/static/content.focusing.js
Outdated
| // Try aligning the right edge of the label to the right edge of the | ||
| // border. | ||
| // | ||
| // If the label is too wide, align the left edge of the label to the |
There was a problem hiding this comment.
"too wide" actually means "goes off-screen left"
matatk
commented
Jun 4, 2018
src/static/content.focusing.js
Outdated
|
|
||
| const settings = {} // caches options locally (for simpler code) | ||
| let labelFontColour = null // computed based on border colour | ||
| const contrastChecker = new ContrastChecker() | ||
|
|
||
| let currentBorderResizeHandler = null | ||
| let currentlyFocusedElementInfo = null // keep for border redraws | ||
| let currentlyFocusedElementBorder = null // indicates border; resizing |
There was a problem hiding this comment.
Rename these to indicate they're elements themselves?
* Remvoe the now-unnecessary scrollWidth checks. * Rename variables for clarity. * Re-order declarations and tidy up the formatting.
* Tidy up side-scrolling test page (make it fit vertically nicely). * Add a test page for viewport-filling regions in both dimensions.
matatk
changed the title
* Ensure that the border and label are drawn within landmarks@ bounding boxes. * Resultant minor code tidy-ups. * Switch the border and label to border-box sizing. * Fixes #167.
Use document.documentElement.clientWidth as this takes the width of the scrollbars (if rendered) into account, whereas window.innerWidth did not.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This moves labels if need be to ensure they’re always visible. It also decouples the label from the border, to enable that re-positioning and to stop the “split borders” effect.
This also allows pointer events (such as mouse hover) to pass through the landmark border
<div>so that the user can interact with the web page as normal.Borders are drawn inside the space required by elements, to avoid introducing scrollbars when full-width/height regions are present.
Fixes #166
Fixes #175
Fixes #167