Problem/Motivation

Steps to reproduce

Proposed resolution

Remaining tasks

  • ✅ File an issue about this project
  • ☐ Addition/Change/Update/Fix to this project
  • ☐ Testing to ensure no regression
  • ☐ Automated unit/functional testing coverage
  • ☐ Developer Documentation support on feature change/addition
  • ☐ User Guide Documentation support on feature change/addition
  • ☐ Code review from 1 Drupal core team member
  • ☐ Full testing and approval
  • ☐ Credit contributors
  • ☐ Review with the product owner
  • ☐ Release

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#13 pagey.png31.27 KBbnjmnm
#12 Screen Shot 2022-06-08 at 12.08.27 PM.png11.7 KBhooroomoo
#12 Screen Shot 2022-06-08 at 12.02.38 PM.png11.76 KBhooroomoo

Issue fork project_browser-3282707

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

hooroomoo created an issue. See original summary.

hooroomoo’s picture

hooroomoo
they/them
Location nyc🗽
commented
Issue tags: +Accessibility
bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Assigned: Unassigned » bnjmnm
libbna’s picture

libbna commented

Hi @bnjmnm are you working on this issue?

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented

Yes

bnjmnm opened merge request !129

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Assigned: bnjmnm » Unassigned
Status: Active » Needs review

Ready for review. I'm trying something out in the MR where I explain each of my changes -- stuff that makes it easier to review but would be wildly confusing as a comment in code (such as explaining why something was removed // This is why you're not seeing a 20 item array here, ok?). This goes a little further than the IS, but the result is better accessibility, easier to navigate pagination code, and removed a bit of unused code.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Title: Pagination is missing role="navigation" » Pagination accessibility improvements

Note that I left a bunch of comments explaining my own work on the MR which should make this easier to review.

hooroomoo’s picture

hooroomoo
they/them
Location nyc🗽
commented
Assigned: Unassigned » hooroomoo
hooroomoo’s picture

hooroomoo
they/them
Location nyc🗽
commented
Assigned: hooroomoo » Unassigned

Start looking at this, will continue tomorrow but unassigning myself for now in case anyone else wants to looks at this

hooroomoo’s picture

hooroomoo
they/them
Location nyc🗽
commented
Assigned: Unassigned » hooroomoo
hooroomoo’s picture

hooroomoo
they/them
Location nyc🗽
commented
Assigned: hooroomoo » bnjmnm
Issue summary: View changes
Status: Needs review » Needs work
StatusFileSize
new Screen Shot 2022-06-08 at 12.02.38 PM.png11.76 KB
new Screen Shot 2022-06-08 at 12.08.27 PM.png11.7 KB

Accompanying comments were very helpful. Looks good and the new PagerItem component makes it much cleaner.

Just one nit, I think the spacing between the "First"/"Last" and the arrow icon regressed, theyre very close to each other now. Fine with it being a followup and I can RTBC or i think this needs a rebase anyway so if the spacing could be added then.

MR:

HEAD:

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Needs work » Needs review
StatusFileSize
new pagey.png31.27 KB

Good catch! PagerItem needed a bit of additional styling.

hooroomoo’s picture

hooroomoo
they/them
Location nyc🗽
commented
Assigned: bnjmnm » Unassigned
Status: Needs review » Reviewed & tested by the community
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Issue tags: +Project Browser MVP

  • tim.plunkett committed c9d8654 on 1.0.x authored by bnjmnm 
    Issue #3282707 by bnjmnm, hooroomoo: Pagination accessibility...
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Reviewed & tested by the community » Fixed

Merged, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.