- 🇮🇳India Shubham Sharma 77
Fixed failed commands on #74 against 10.1.x
Attached reroll patch against Drupal 10.1.x - Status changed to Needs work
almost 2 years ago 5:13pm 26 January 2023 - 🇺🇸United States andy-blum Ohio, USA
Patch #76 causes some problems which seem to be related to using inset: 0;
On a fresh installation with patch #76, we now have a horizontal scrollbar, and visually hidden elements still have a width/height within the viewport. The element in the screenshot below is a contextual link that has some .visually-hidden rules overridden.
If we modify the inset values back to "-100vh auto auto -100vw", however, the horizonal scrollbar disappears, and our element no longer takes up space within the viewport.
- 🇺🇸United States andy-blum Ohio, USA
Doing some additional digging, the exact element causing this overflow appears to be the contextual links button for the Olivero search block. That block has it's styles deliberately overriding position:relative, causing the contextual link button's offsetParent to be the header, and it's positioned absolutely at the edge of the viewport.
While this is an Olivero-specific issue, I still think we ought to be using the styles set in #71 and adjusting the tests as needed.
- 🇺🇸United States bnjmnm Ann Arbor, MI
Patch #76 should be ignored.
Working from #74, I reviewed a subset of the failing tests and they all appear to be due to .visually-hidden now being too hidden for our PHP tests to access. For example, I couldn't find any way to get
#drupal-live-announce
contents with a PHP call, but fortunately it's still available if we use JS to get it so$this->getSession()->evaluateScript("document.querySelector('#drupal-live-announce').textContent");
still gets us what we need. Methods such as
$page->find()
or$assertSession->elementTextContains()
were not able to find the element. It's possible there's a test method that can do this I'm not aware of, but the->evaluateScript()
is an option I know will work.Given that our PHP tests can't see these .visually-hidden elements, we need to confirm they aren't hidden from Assistive Tech. I confirmed it is fine with Voiceover on all modern OSX browsers, but this will need to be checked with Narrator, NVDA, and JAWS on Windows too. A confirmation that Drupal.announce()/
#drupal-live-announce
will be sufficient. - First commit to issue fork.
- 🇺🇸United States bnjmnm Ann Arbor, MI
@VladimirAus I'm closing your merge request to avoid confusion that might arise from the assumption that the most recent activity is the most up-to-date, but in this case it's taking the solution back to an approach that was decided against over a year ago in #39. It's great to have people participating in issues, but if it's done without reading the full history of the issue (which can be a long read), the changes can be disruptive and hurt the chances of it ever being completed.