Skip to content
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

Ensure the Page chooser modal listing uses buttons #9209

Closed
wants to merge 1 commit into from

Conversation

lb-
Copy link
Member

@lb- lb- commented Sep 16, 2022

Details

  • Clean up some commented out css & also split out specific scss ignore lines in _listing.scss, the goal here being to clean up a little bit of this file but basically make the button look exactly the same as the a (link) variants used everywhere else.
  • Add proper aria-description (intentionally not aria-label) to provide extra context for this button's purpose aria-description="{% trans "Select page" %}".
  • Use classnames template tag for simpler classes generation.
  • Note: The issue had an additional call out to change the way hover behaviour works (hover only on the clickable targets, I gave this a go but honestly the DOM structure & CSS here is pretty fragile and I opted for a smaller fix for the core issue before we start re-working everything.
  • If this is merged, I will raise a new issue for the styling of the clickable buttons in page listings, we may want to add an explicit 'select' button (similar to full sized page listings) but this will require some additional thought.
  • If this is merged, I will raise a new issue to adopt this solution for other chooser listings (images, document, snippets) along with the 'children' button.
    • wagtail/images/templates/wagtailimages/images/results.html
    • wagtail/images/templates/wagtailimages/chooser/results.html
    • Potentially others

Checklist

  • Do the tests still pass?
  • Does the code comply with the style guide?
  • For front-end changes: Did you test on all of Wagtail’s supported environments?
    • Safari 15.4, Firefox 104, Chrome 105 including VoiceOver
    • Edge with NVDA

Screenshots

Screen Shot 2022-09-16 at 5 15 22 pm

Screen Shot 2022-09-16 at 5 17 16 pm

@lb- lb- added status:Needs Review component:Frontend Accessibility component:Choosers Modal choosers for Page, Snippet and other models labels Sep 16, 2022
@squash-labs
Copy link

squash-labs bot commented Sep 16, 2022

Manage this branch in Squash

Test this branch here: https://lb-fix5408-page-chooser-use-bu-kcw2u.squash.io

@lb-
Copy link
Member Author

lb- commented Nov 28, 2022

Rebased

@lb- lb- force-pushed the fix/5408-page-chooser-use-buttons branch from e59ee2a to a777807 Compare November 30, 2022 10:14
@lb- lb- force-pushed the fix/5408-page-chooser-use-buttons branch from a777807 to f850b5a Compare January 11, 2023 10:00
@lb- lb- force-pushed the fix/5408-page-chooser-use-buttons branch from f850b5a to f1c68f1 Compare March 1, 2023 11:37
@lb-
Copy link
Member Author

lb- commented Mar 1, 2023

Rebased

- add proper aria-description (intentionally not aria-label) to provide extra context for this button's purpose
- use classnames template tag for simpler classes generation
- clean up some commented out css & also split out specific scss ignore lines in _listing.scss
- fixes wagtail#5408
@lb- lb- force-pushed the fix/5408-page-chooser-use-buttons branch from f1c68f1 to 8b00733 Compare March 1, 2023 11:43
@lb-
Copy link
Member Author

lb- commented Apr 3, 2023

Will close this for now, been over 6 months of rebasing / fixing conflicts - obviously not a big priority at this time. I will comment on the original issue that anyone is free to pick this up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Page chooser should use buttons instead of links with invalid targets
1 participant