Problem/Motivation

Recipes do not really have a concept of "number of active installs" and I don't think we'll necessarily have a way to record installs for general projects, if that's what they end up being on d.o.

Proposed resolution

I think it makes sense to solve this issue by only showing the number of installs if it's non-zero, at least for now.

I think this can easily be handled with a Svelte if statement:
https://git.drupalcode.org/project/project_browser/-/blob/2.0.x/sveltejs...

CommentFileSizeAuthor
#5 after.png336.3 KBsnehal-chibde
#5 before.png283.63 KBsnehal-chibde

Issue fork project_browser-3461037

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

chrisfromredfin created an issue. See original summary.

sourav_paul’s picture

sourav_paul
Location Kolkata
commented
Assigned: Unassigned » sourav_paul

Sourav_Paul opened merge request !557

sourav_paul’s picture

sourav_paul
Location Kolkata
commented
Assigned: sourav_paul » Unassigned
Status: Active » Needs review

Please review the MR.

snehal-chibde’s picture

snehal-chibde commented
StatusFileSize
new before.png283.63 KB
new after.png336.3 KB

Hello @sourav_paul, I have reproduced the issue successfully.
I have tested MR 557 on the Drupal 10.3.1 version, Module version 2.0.x-dev.
The MR is applied cleanly.

Test Result:
If a recipe has 0 installs, no span is added with text as '0 installs' which was appearing before.
Added a before and after screenshot of the same.

Moving this to RTBC state.

snehal-chibde’s picture

snehal-chibde commented
Status: Needs review » Reviewed & tested by the community
bramdriesen’s picture

bramdriesen
he/him
Primary language Dutch
Location Belgium 🇧🇪🇪🇺
commented
Status: Reviewed & tested by the community » Needs work

Merge conflict in the generated file, needs to be re-compiled.

Shouldn't the IF statement only work on recipes? I would imagine for regular modules/themes it's valid to know there are 0 installs.

prashant.c’s picture

prashant.c
he/him
Primary language English
Location Dharamshala
commented

@BramDriesen

Currently, I think the recipes are being fetched from the local directory only which is core/recipes and if that is the case then the number of installs would be either 1 or 0, which will not make sense in my opinion.

Therefore I think it a good idea to not show this section at all in case of Recipes.

bramdriesen’s picture

bramdriesen
he/him
Primary language Dutch
Location Belgium 🇧🇪🇪🇺
commented

Therefore I think it a good idea to not show this section at all in case of Recipes.

Yes sure, but I don't see that distinction being made in the code changes. I only see a counter check if 0 to hide it, meaning in my eyes this is for everything and not only recipes.

chrisfromredfin’s picture

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

I actually would prefer that the behavior be the same across all, not just for recipes.

chrisfromredfin’s picture

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

nice little UI improvement

Status: Fixed » Closed (fixed)

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