Problem/Motivation

We are currently using a convention to dictate meaning as we treat as "logo" the first image available in the images field.
This convention could be hidden from the code and encapsulated in the mock, thus not needing to shift arrays or do different checks in the front-end.

Related conversations about it can be found here #3277464: [PLAN] Where to fetch image from?

Steps to reproduce

See how the front-end makes use of the first image in "field_project_images" for logo, and the others for the carousel, needing comments to actually understand what's going on.

Proposed resolution

Make sure that the project class returns "logo" as a separate property to "field_project_images" and leave the plugins decide where the logo comes from (ie: the mock can still decide to make the logo the first image).

Remaining tasks

  • ✅ File an issue about this project
  • ☐ Manual Testing
  • ☐ Code Review
  • ☐ Accessibility Review
  • ☐ Automated tests needed/written?

Issue fork project_browser-3301577

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

fjgarlin created an issue. See original summary.

fjgarlin’s picture

fjgarlin commented
Issue summary: View changes
Related issues: +#3277464: [PLAN] Where to fetch image from?

fjgarlin opened merge request !219

fjgarlin’s picture

fjgarlin commented
Assigned: fjgarlin » Unassigned
Status: Active » Needs review
fjgarlin’s picture

fjgarlin commented
Assigned: Unassigned » fjgarlin
Status: Needs review » Needs work

Need to rebase/refactor based on the latest commit to 1.0.x

fjgarlin’s picture

fjgarlin commented
Assigned: fjgarlin » Unassigned
Status: Needs work » Needs review

Rebased and adapted to the new expected format. I'm fixing a small regression too as part of this MR too.

fjgarlin’s picture

fjgarlin commented
Status: Needs review » Needs work
fjgarlin’s picture

fjgarlin commented
Status: Needs work » Needs review
fjgarlin’s picture

fjgarlin commented

For better testing, enable both the mock and the random data plugin ("Project Browser Devel" module) as they both have different strategies for the logo.

Everything should still work in the same way, but the svelte code is hopefully clearer and we don't need to make assumptions about where the logo is as that will be dealt with by the plugin.

bnjmnm’s picture

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

Left some feedback on the MR.

This is a great improvement, this will address a confusing part of the Svelte logic.

fjgarlin’s picture

fjgarlin commented
Status: Needs work » Needs review

Thanks so much for the review and suggestions @bnjmnm, I really like the simplifications on the svelte files.
I've addressed them all and pushed the changes.

bnjmnm’s picture

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

Looks good to go!

tim.plunkett made their first commit to this issue’s fork.

  • tim.plunkett committed f0e706c on 1.0.x authored by fjgarlin 
    Issue #3301577 by fjgarlin, bnjmnm: Have a separate field/property for...
tim.plunkett’s picture

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

Some minor cleanup before merged, but this was great! Thanks

Status: Fixed » Closed (fixed)

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