forked from davidtodd/landmarks
-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Robust, customisable borders with labels #158
Merged
Merged
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
Owner
matatk
commented
May 8, 2018
- Draw borders as elements on their own, including a label that gives landmark info (closes Labelling of Landmarks #1, fixes Is there a way to make the CSS outline always visible? #47).
- Customisable border colour.
- Customisable border label font size.
- The label’s font colour is automatically selected to be black or white depending on the colour of the border.
* Make border elements so that they can be positioned on top of all other elements. * Factor out landmarkName() to a separate file as it is used to provide a label for the border effect. * Update the manifest and injector script in line with this new file. * Make the functions that returned elements now return the role and label too (so that they can be used in the label for the border). * Try to ignore changes to the DOM due to adding our border elements.
* Take out code that ignored border mutations, as they should be ignored by the new ignoring mechanism. * Provide a mechanism for ignoring border related mutations (ask the ElementFocuser if it just made changes). * Rename variables for clarity. * Reinstate the line that allowed the currently-selected landmark to be tracked across landmark updates. * Fix moving to the main landmark.
* Improve function name. * Improve variable names and spacing.
* Cancel the timeout that removes the border if another element is highlighted. * A dictionary was not necessary for storing element borders as there is only one! (There may well be more than one later, when all are added at once, but that is getting ahead of oneself.) * Add an option for border colour. * Add a 'defaults.js' file to store options. The injection and number of support scripts is a bit big now - time to consider rollup or similar?
* Add a setting for border label font size. * Simplify the handling of defaults.
* Add a label font colour option. * Apply the "browser-style" class as per MDN advice (<https://developer.mozilla.org/en-US/Add-ons/WebExtensions/user_interface/Browser_styles> —was missing the class on parents of <input>s).
The "(pixels)" bit wraps in Chrome, but not Firefox.
* Add a build:all:quick script (no cleaning or testing done first). * Fix a bug in how options changes for the logger were not handled correctly. * Handle options changes in the focuser in the same way as in the logger.
These will be used to determine the appropriate label font colour.
* Add tests for contrast checking. * Add a function to work out whether the label should be black or white for a given border colour (not yet wired up). * Add a TODO for tidying up function definition in focuser.
Instead it works it out (black or white, at least) :-).
matatk
added a commit
that referenced
this pull request
May 10, 2018
This resets various CSS properties that are not explicitly used by the border and label elements, in case the page does set them. It also removes a redundant data attribute that was used early on in #158 but a better way was found to ignore the borders so it's no longer needed.
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.