Overview

#3459235: Implement front-end (React) routing library introduced a critical regression: the prop labels no longer show up in the form in the right sidebar. This was working fine since #3461422: Evolve component instance edit form to become simpler: generate a Field Widget directly.

#3461422: Evolve component instance edit form to become simpler: generate a Field Widget directly introduced test coverage, and it is explicitly asserting the presence of labels. BUT it's asserting that using HTML markup — it's not asserting that those are visible.

Proposed resolution

  1. Expand test coverage to also assert visibility.
  2. Determine root cause + fix.

User interface changes

Before (broken)
After
labels visbile
CommentFileSizeAuthor
#5 Labels_visible.png65.14 KButkarsh_33
Screenshot 2024-08-01 at 9.39.46 AM.png533.61 KBwim leers

Issue fork experience_builder-3465140

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

Wim Leers created an issue. See original summary.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +Usability

Utkarsh_33 made their first commit to this issue’s fork.

Utkarsh_33 opened merge request !133

utkarsh_33’s picture

utkarsh_33 commented
Issue summary: View changes
StatusFileSize
new Labels_visible.png65.14 KB
utkarsh_33’s picture

utkarsh_33 commented
Status: Active » Needs review
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: Unassigned » bnjmnm
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

Thanks, @Utkarsh_33!

So, turns out that the labels were visible, they just had no contrast at all 😅🤣

AFAICT that means that the to.be.visible assertion you added would have passed already anyway; in theory we should test that the contrast is sufficient, but that seems unnecessarily detailed.

I think this is fine as-is, but deferring to @bnjmnm, our accessibility expert, and also the primary author of this functionality 😊

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Assigned: bnjmnm » Unassigned
Status: Reviewed & tested by the community » Needs work

Left feedback on the MR

utkarsh_33’s picture

utkarsh_33 commented
Status: Needs work » Needs review

  • bnjmnm committed c8eb44a3 on 0.x authored by Utkarsh_33 
    Issue #3465140 by Utkarsh_33, bnjmnm, Wim Leers: Regression: prop labels...
bnjmnm’s picture

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

All set.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Nice, a bug fixed by only deleting code 😁

Status: Fixed » Closed (fixed)

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