Yeah I'll work on this in the 3.x branch. That will have a beta period that will be good for feedback.
itmaybejj → created an issue.
Good feedback.
The proposed change is almost sounds awfully like something I iterated through during development. I went rooting around and found these early screenshots:
I thought the arrows were ambiguous -- they seemed unclear as to whether they would "dock/close the interface" or "go to next issue." I myself switched what they did a few times in those iterations. It also wasn't clear that the number was a count of "problems on the page" rather than, say, message notifications -- it seemed too easy to ignore. I gravitated towards having the number and the alert flag visible because I thought it was more eye-catching to have the flag come and go.
Another consideration...you can open and shut the checker even when there is no flag, so it seemed natural to use main toggle as the "next" rather than the number; otherwise I would need a third "power" icon.
Although in looking through old screenshots...I do not know if I explored another direction -- switching the position of the number and the flag. That would let the number increment to the next issue. Roughly mocking that up now, it could look something like:
...thoughts?
Hopefully 1.0.17 solves this for you → .
Publishing your solution as a sidecar module is PERFECT, because it introduces some dependencies that I would not want in the main module. This is where I was thinking of going myself with other integrations.
I figure what I'll do is edit the project page to highlight this and other "connectors" as they appear.
One thing you may want to consider is letting authors dismiss these alerts if SiteImprove is wrong -- if you wanted to do that, you would add a dismissal key when you push the result. Instead of:
dismissalKey: false,
you would use:
dismissalKey: Ed11y.dismissalKey(el?.getAttribute('href')),
Config to turn off checker while editing should work correctly in 2.2.9 → .
itmaybejj → created an issue.
Oh goodness; neither config is working right any more. I will try to get a release out tomorrow with bugfixes.
Yes; I agree that this is confusing in the UI at the moment.
It looks like it would be possible (though complicated) to modify the library to check inside closed tabs provided by the Field Group module → , as that module does render the field content in the DOM even when hidden. I would need to modify some of the parameters at runtime so that it does not ignore these hidden elements, would need to add a function to hide the tip toggles from results in any hidden tabs, and add a function to ask Drupal to switch tabs and then rerender my results with a different set of hidden tip toggles if someone hits "next" into an alert on the hidden tab.
It looks like it would not be possible to modify the library to check inside closed Paragraph → tabs, as Paragraphs does not render the DOM until a tab is opened.
I'm...honestly not sure what makes sense to do here. I'm trying to be consistent, so I do not like the idea of the module working notably different depending on the fields present on a content type. And Princeton does not put CK content inside Field Group tabs, so I am not sure they would want to sponsor the amount of work involved in something for only that use case.
Alright; I'll have to add a config field for selectors for elements to not receive the Nobreak. That should be easy enough, and parallel the config fields for links to ignore or links to not show the icon at all. I don't see any practical way to make a Nobreak compatible with someone applying a CSS gap on the contents of a link.
But this CSS gap declaration -- where is it coming from? I don't see it in Olivero, only Gin. I like to support common themes/modules out of the box.
I'm afraid this is controlled in the user's browser preferences, and is not a feature of HTML. Links specify they want to open in a blank context, and the browser decides if that will be a new tab or window.
It is possible for JavaScript to attempt to force a new window, but this is such unexpected behavior that browsers block it and throw a toolbar alert asking a user to permit the site to open popups.
Now...it's been a few years since every browser make switched the default from Tab to Window, and very few people know they can go into their browser preferences to change it back, so perhaps I should change the default wording in the config to Tab instead of Window...
ack; will fix
Oofta; found a real issue in the wild. The caps lock test is escaping confinement.
Summarizing notes from Slack conversation --
VBO does an odd thing where it has an unlabelled checkbox with a title attribute as a pseudo-label. This is...not a best practice...but is technically permitted under the accessible name computation. So this is a false positive; the VBO markup is OK.
The challenge here is that the accessible name computation has a lot of steps, and running it all on every element on a page would introduce jank, so Editoria11y and Sa11y only run a subset of its possibilities, based on what is generally possible to create in CKEditor and Gutenberg.
I need to ponder this for a bit and decide whether it makes more sense to add this to our computation, or just add default config to automatically ignore VBO tables. I have seen a similar issue in some Webform tables.
For the moment, users can add VBO table classes to their Editoria11y config for elements to ignore. Editoria11y is only testing tables for headings and headers, so this is unlikely to create any false negatives.
That was my assumption as well, but 🐛 Allow PHP 7.4 when using 2.x Fixed have shown me 🐛 Error: Class "Drupal\Component\Utility\DeprecationHelper" not found in editoria11y_page_attachments Active .
I'll have a branch tagged with this by 🌱 Drupal 12.0 compatibility planning Active , when modules are expected to begin preparing for Drupal 12.
I am hoping this is the last time I need to revisit this for the Gin toolbar and the Drupal modal, but I absolutely expect this issue to resurface with other modules. This time I left a lot of notes-to-self, in hopes I don't break Gin again when I address the next round of surprises.
This change breaks compatibility for 128,000 core installs → (9.x through 10.2). I can't do that in a minor point release.
I am keeping this in postponed until I release a breaking release with new version requirements or Drupal 12 alpha is tagged, whichever comes first. I hope to be ready for a breaking release thus summer or fall, with new DB structure and views integrations.
That...should work.
I added an event to the library for when tips are drawn, and use that to iterate the results and modify their z-index. Now only the Editoria11y toolbar, active tips, and tips on content in a modal should draw on top of the Gin toolbar.
And since I was revisiting Drupal's modals anyway, I added some logic to detect when Drupal opens a modal, which temporarily overrides the check roots to only check within the modal and then re-runs the checker. I figure there's no sense tagging things behind the modal, since you can't get to them until the modal closes. Maybe someday I'll generalize that for any opening modal dialog.
I'll wait to tomorrow to tag a release.
Oh dear you're right. I have competing problems I'm trying to solve here. I'll reopen/revisit this.
3521111 is unrelated; that just added the right-side toolbar to the selector list, so that the Editoria11y toggle will pin to the Gin toolbar rather than the left edge if someone sets it to pin left instead of right.
Pulling out of postponed to integrate into spring/summer work.
Pulling out of the postponed queue to add this to my spring/summer task list.
itmaybejj → created an issue.
If you would like to recheck, I have merged in some recent changes and fixed some issues in the CSS and API.
Oh good; that's what I was hoping because all other possibilities were going to be "exciting" to try to troubleshoot.
itmaybejj → created an issue.
Will do; thanks.
I don't... understand... 2.2.6 and 2.2.5 are identical; same library files, no changes to the module PHP or JS, only some missing strings in the translation file for non English language sites and a change to the dev scripts.
Is it possible there's a race condition here (possibly in my module) or a JS file load order issue, where rolling back is a red herring and it's just that clearing cache as part of the rollback hides the problem? Like, if you clear cache a few times and hard refresh is the problem intermittent in both versions?
Hmm....unless there is a bug, if "Show tips automatically when issues are detected" is set to anything other than "Assertive" (the default), it will only auto-draw the tips if there are new errors on the page. Themers repeatedly refreshing a page should only have stuff tossed all over their work once if they close the toggle and refresh. Subsequent loads will read their local preference for "shut," see that the number of issues hasn't changed, and start minimized (e.g., as an on-demand check). Maybe try setting that to "On first detection" or "Polite" during development and see if that satisfies your themers? "On first detection" used to be the default, but too many editors were ignoring the toggle...
I would consider a merge request to add a per-user preference for "Show tips automatically when issues are detected."
I...don't plan to contribute my own time to this -- if it's modifying inline DOM (other than the toggle), there's presumably a problem on the page. And if that is breaking a component, the themer needs to account for it, since it will break it for front-end editors. And if the toggle is in their way, it's going to be in the editors' way too. So I don't let our themers turn it off; I want them to be putting as much thought into the editor UI as the frontend.
As of the latest release they can hide all duplicates of a manual check on a page for themselves. They could also split out their account to a role that does not have the checker enabled. Or they could temporarily add .ed11y-element {display: none;} to the CSS...assuming they don't accidentally commit the change...
Or they could throw this in dev JS to let them modify anything for themself at runtime, ideally wrapped in an if for drupalSettings.user.uid:
// Tell Editoria11y to call our option override.
var editoria11yOptionsOverride = true;
var editoria11yOptions = function (parameterBag) {
let options = parameterBag;
// Dev approach 1: do not draw interface at all.
// This overrides the GUI config for "Disable the scanner if these elements are detected."
options['preventCheckingIfPresent'] = '.some-devel-button-that-only-themers-see';
// Dev approach 2: ignore config and force polite mode.
options['alertMode'] = 'polite'; // Override any 'assertive' config.
options['userPrefersShut'] = true; // Override localStorage for open/closed state.
// Dev approach 3: only run on click:
options['ignoreAllIfPresent'] = '.some-devel-button-that-only-themers-see';
// That's a library param that isn't in the GUI, the opposite of the GUI one:
// "Hide all alerts if none of these elements are present."
return options;
}
If they are the only themer on the project, they could also just add ".some-devel-button-only-themers-see" to the GUI config for "Disable the scanner if these elements are detected."
Mostly...I just philosophically do not think putting something in the GUI that lets end-users to turn the checker on and off is a good idea; this is one of the key differentiators between Sa11y and Editoria11y. Sa11y allows more local-user control, Editoria11y is centrally managed. I found that users who could turn things off, did. And then they forgot to turn things back on.
I would consider a merge request if it let the admins control who was able to do this.
Oh goodness that should have been caught in testing; the title string never got written for that field.
Those boxes are for tests to turn off. I'll fix that in the next release.
I don't plan to add an include/exclude toggle, just because tests come and go and various sites have custom tests, so a simple "ignore" param is much easier to manage in the library. I'd consider a clever merge request if one came through.
That's a good idea. I'll track my work in #3521111.
timwood -- I'd recommend making the same feature request in back-to-top. Because that displays late I've seen it have issues with other modules. They are welcome to yoink my code.
Sure I can add this in the next release. I already added the param to the library recently, so I just need to expose it on the config page.
It already does! just swap the filenames or contents of editoria11y.libraries.dev.yml and editoria11y.libraries.yml during development -- the dev.yml library definitions are for the uncompressed/unpackaged files, which can be found in /library/js/
Fair enough. The functionality is by design, but maybe I should revisit the design in a future iteration so there is a visual distinction between the editable headings and the template headings. Though in a lot of contexts I just won't know which it is; there are often a lot of template headings within the editable area.
Is it flagging things as errors outside the specified check areas, or just showing other headings in the little visualizer?
What I am trying to do at the moment is have the page outline visualizer ignore the check roots and show all headings, but for errors in headings to only be flagged if they are within the specified check roots. But that means I'm running the same DOM dive two different ways and have to keep track of which tree I'm using at all times, so errors on my own part would not be surprising...
Indeed.
I think the best thing here because of the late fade-in is to use CSS to move the back to top widget instead, so you don't get a weird floatie-thing going where Editoria11y is sorta vaguely centered before the other widget appears. Something like:
body:has(ed11y-element-panel) #backtotop {
margin-right: 140px;
}
I'm a little loathe to add that sort of thing in my module, since I'd be moving their widget; might be good to ask Back to Top to add an offset parameter?
Ahh yeah; I'm afraid that explains it. Injecting my code into the CK4 iframe was not feasible.
I'll convert this to a documentation task then, so I don't forget to make this clearer in the next release.
It should check within CKeditor5 on a node edit form, but it should not appear on the admin theme unless a CKeditor5 instance is present and the Layout Builder editor is not currently upen. It is not compatible with CKeditor4 or the Layout Builder editor.
The module readme appears to be out of date. Checking while editing rolled out right before DrupalCon.
If you are not seeing it on a node edit form with a CKeditor5 instance on screen, using stock configuration, I'll have some followup questions as there may be some new module conflict I'm not aware of.
🎉
Huzzah!
Link indication contrast in body content
Got a WCAG issue that shouldn't be too hard to solve -- the links in body content (module info pages, these comments, etc) are hard to see because they only have a 2.8:1 contrast ratio against their surrounding text. 3.0:1 is the minimum a lot of people can see:
This in theory could be fixed on a white background by using a lighter blue, but that's just not going to work because the same link color is used on gray backgrounds (e.g., the "Issue Metadata" box), and a lighter blue will then fail contrast on the background.
So the typical solution is to make the links stand out using a secondary indication -- e.g., underline:
#main a[href]:not(.button) {
text-decoration: underline;
}
#main a[href]:not(.button):hover, #main a[href]:not(.button):focus-visible {
text-decoration-style: double;
}
Or bold:
#main a[href]:not(.button) {
font-weight: bold;
}
Yup it looks like your theme is inserting a gap between elements, so adding elements adds gaps:
.menu .primary-nav__menu-link > span {
gap: 0.5rem;
}
This would fix it:
.menu .primary-nav__menu-link.link-purpose > span {
gap: 0;
}
So; that extra markup is most of what this library does, rather than CSS ::after pseudocontent. The span wrapper prevents the link from breaking its line between the last word and the icon. But yes, sometimes themes need to style the icons to make the spacing match the text in menus, especially the icon width and height. This is the CSS provided by the library.
If you provide me a link I can take a look.
the_g_bomb → credited itmaybejj → .
I'm flagging this as fixed for now because of testing on a copy of your DOM. Please test 2.2.5 → with the new param "Auto-detect any Web components" turned on under "Web components and custom tests," and reopen if I haven't succeeded.
I have that param off by default for now until I'm confident it both works and doesn't hurt performance. This part of the code usually finishes in less than 1ms so I don't think it's going to hurt performance in any noticeable way.
Switching this to Fixed because I think 2.2.5 → will prevent future folks having the same headaches.