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

Issue fork project_browser-3282692

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

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

narendraR opened merge request !128

narendrar’s picture

narendrar
He/Him
Primary language English
Location Jaipur, India
commented
Status: Active » Needs review
chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Status: Needs review » Needs work

I'm not sure what I'm supposed to be looking for here. With the patch applied, I can see that the placeholder image always has an alt of "Placeholder" and the images are showing their alt, i.e. "webform logo" - but Pathauto has an empty alt. Does this code do this?

* use the alt attribute provided by d.o
* if not found, use an empty alt
* use "placeholder" for all the puzzle piece images?

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Status: Needs work » Reviewed & tested by the community

I think we pieced this together in Slack (timplunkett, bnjmnn, and myself) so I think this actually is the right thing here.

bnjmnm’s picture

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

It looks like the MR is a bit different from what was discussed. I believe we settled on marking all images as decorative with alt="". I provided additional explanation in the MR.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

Agreed with #8, the MR came before the discussion.

narendrar’s picture

narendrar
He/Him
Primary language English
Location Jaipur, India
commented
Status: Needs work » Needs review
bnjmnm’s picture

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

Looks great!

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented

I rebased and am waiting for tests, as there was a svelte build failure for a no-longer-used projectTitle prop.

tim.plunkett’s picture

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

NW for the slight nit, sorry

narendrar’s picture

narendrar
He/Him
Primary language English
Location Jaipur, India
commented
Status: Needs work » Needs review

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

This was RTBC before, so I'm merging it. Thanks for the fixes @narendraR

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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