🇮🇳India @narendraR

Closing this issue as Closed(cannot reproduce),

The changes look good to me. We can improve them later if needed, but I think this is good to go for now.

This MR needs re-roll.

Tested manually and focus is now set on modal. Moving it to RTBC.

Looks good to me.

It seems that the cache is already being cleared during tests, which is why this issue is not reproducible. You can proceed with the fix as suggested at https://git.drupalcode.org/project/project_browser/-/merge_requests/621#... and remove the test for now.

If necessary, a follow-up can be created to investigate the test further.

Below are some findings:

• Still shows 1 item selected
• I think on clicking Install selected projects when no project is selected, it should scroll to top where the error message is shown
• Cursor : pointer in select button
• I am not sure if color of Install selected projects and Select/Unselect is correct or not.

Changes looks good to me. Thanks for working on this issue. Moving it to RTBC.

This MR needs rebase and a change needs to be done at one more place.

This looks goos to me, except that on token list/grid it is showing as 590949 Active Installs, but it should be 590,949 Active Installs, so this also needs to be changed accordingly.

I thin numberFormatter is required on both modal and listing page to display numbers formatted correctly.
Can we do something like:

        {Drupal.formatPlural(
          project.project_usage_total,
          `${numberFormatter.format(1)} site reports using this module`,
          `${numberFormatter.format(project.project_usage_total)} sites report using this module`
        )}

Moving it back to NW for #20, unless someone thinks otherwise.

Asserting focus in exiting modal test can be helpful here.

Tested manually and here are some suggestions:

• 0 items selected ==> No projects selected
• Install selected projects button should be disabled if no project selected
• Bulk action banner should be sticky as soon as some project is selected
• Remove unnecessary added classes
• #5 Needs to be addressed

All feedback addressed, moving it to RTBC.

Looks good to me. Just a small suggestion along with creating a new issue to replace queue/dequeue references.

Changes looks good to me. Moving it to RTBC.

Regenerated fixture on local using php scripts/regenerate-drupalorg-jsonapi-fixture.php and run all tests in ProjectBrowserUiTestJsonApi. All tests are passing after fixture update.
Moving this issue to RTBC.

Changes looks good to me.

Hi @nidhish, are you working on this issue?

Looks good to me. Tested manually and it is working as expected.
Should we also consider fixing 1 sites report using this module on modal in scope of this issue?

Looks good to me, except some minor suggestions.

https://www.drupal.org/drupalorg-api/project-browser-filters → should be used to get maintained values. Also do we have some test to verify the wrench icon on listing and detail page?

install-state.gz should not be part of MR.

✨ Consider making 'detail page' a modal window instead of a separate "page" in app Active is in. This MR needs to be updated accordingly.

Images are visible in modal, marking as RTBC.

This looks good, adding test coverage could be beneficial here.

Changes looks good to me. Just to add that if changes are not reflecting change the order of the enable source plugins or add disabled one.

Issue reported in #18 will be fixed in separate issue

narendrar → created an issue.

I am not seeing images for field_group, token, metatag.

This issue requires a new endpoint in DrupalOrgClientMiddleware. But filter ID's are changed on DO, so I think an update in fixture is required before working on this issue further. Next steps once fixture is regenerated:

• Add testPagingOptions test in ProjectBrowserUiTestJsonApi.php also, which currently exists in ProjectBrowserUiTest.php
• Add new endpoint in DrupalOrgClientMiddleware.php and use in above test
• Fix nightwatch test.

I think this is still a valid issue as per #3.

Added ✨ Consider making 'detail page' a modal window instead of a separate "page" in app Active as related, as if this issue goes in first, this change also needs to be done in that issue.

Added ✨ Consider making 'detail page' a modal window instead of a separate "page" in app Active as related, as if this issue goes in first, this change also needs to be done in that issue.

Changes are aligned with #22, marking as RTBC.

Thanks @shalini for working on this issue. However this issue still exists after applying your changes.

Steps to reproduce with your MR applied:

• On a fresh install enable Project browser module only and verify that 2 sources are available
• Now enable Project browser devel module and it still shows 2 sources and on configuration page title is still blank
• Clearing cache manually will fix the source title on config page, but it will still show 2 sources

Using below code in .install can fix it.
\Drupal::service('cache.discovery')->deleteAll();

narendrar → made their first commit to this issue’s fork.

Created 🐛 [PP-1] Queue button is not working in modal Active

This issue may require some accessibility review. Feel free to remove tag if this is not the case.

narendrar → created an issue.

How about changing the dropdown text to 'Select more categories' when one or more categories are selected? The dropdown's purpose is clear, and we are already displaying the selected categories as chips.

if it already exists on 2.0.x then would it be so bad to make it its own separate issue so this can go in?

Agreed, we can make it conditional such that if the package manager is available, the queue button is not shown, with a @todo to address it in a new issue.

in what way does it need to be refactored?

I think as we are not using ModulePage now(moved in popup), any code related to it can be removed from App.

Now, we need to click the search icon after entering text, so it seems all points raised in this issue are outdated. Therefore, this issue can be marked as closed (outdated).

One issue I noticed is that clicking the Queue button in the popup does not change it to Dequeue.
This issue also exists in 2.0.x on going to detail page, but as we are removing details page with popup in this issue, I think it should be addressed here.

Changes looks good to me. Marking as RTBC

Both points raised in #5 are resolved.

Switching tabs after visiting individual routes seems problematic (sortCriteria becomes undefined).

Tabs are not shown in case of plugin specific route.

One more problem which I observed is that if someone goes directly to

Removed session storage on mount.

Switching tabs after visiting individual routes seems problematic (sortCriteria becomes undefined).
Steps to reproduce:

• Go to /admin/modules/browse
• Change url to /admin/modules/browse/drupalorg_jsonapi
• Click Recipes tab, sortCriteria is set to undefined and tab will not change
• Go to /admin/modules/browse, loader will appear and no data displayed

narendrar → made their first commit to this issue’s fork.

Changes not implemented as suggested in #17

Found some issues while testing this MR

• Select/unselect a category multiple times and then click Apply. Category is showing multiple times in label.
• Unable to unselect a category from keyboard.
• Clicking Clear does not remove selected categories.
• Unable to select multiple categories after clicking apply.

2 issues I found while testing manually:

• Size of Installed button is not same as other buttons
• Voiceover does not announce this disabled installed button(Not sure if I tested this correctly)

@rkoller, Do you think this issue is ready to be Closed(outdated), or is there anything further that still needs to be addressed?

I tried to explain the fixture generation process in https://git.drupalcode.org/project/project_browser/-/merge_requests/576/diffs#2c53c9f585a22c6bde4e9c11f8b48e8534303873_33_33

Fixture regenerated, added some comments in DrupalOrgClientMiddleware so understand api requests. Moving it to NR.

alexpott → credited narendrar → .

Rebased to latest code.

narendrar → made their first commit to this issue’s fork.

Changes seem in line with comment #23, hence marking it as RTBC.

Suggestion applied and comment added.

🐛 Selected categories disappear on switching tabs Active has been created. The ProjectBrowserUiTest::testMultiplePlugins should be updated to ensure this issue is caught in the first place. I will make the necessary updates and report back.

narendrar → created an issue.

Re #10, It seems an existing issue to me.
Steps to reproduce:

• Enable Drupal core plugin
• On Drupal.org(JSON:API), click clear filters
• Select first 3 categories from category dropdown
• Switch tab to Drupal core plugin and select one category, eg: Core(Experimental)
• Switch tabs in random order to see the issue

narendrar → made their first commit to this issue’s fork.

Re #20, I think placeholder text should change based on plugin or it should be such that it applies to all plugins.

As we now have a list of elements that can be queried, can we decide on naming the placeholder, keeping recipes and future themes in mind?

This is how it is done in ✨ GUI install multiple modules at once. Needs work , so I think this issue can be closed now.

Hi @Chris, Can this issue be closed now?

Hi @Chris, is this issue relevant now as we have JSONAPI endpoint in use or should we close this?

Hi @Chris, can this issue be closed as outdated?

Hi @Chris, is there any decision made on what needs to be done here?

@Chris, can you please close this issue?