@utkarsh_33

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

I think there are a couple of issue which are related to the styling in particular to gin theme.@ phenaproxima. suggested a better way to solve a problem for styling in Change UI for the install queue to match bulk operations 📌 Change UI for the install queue to match bulk operations Active issue's
this comment. 📌 Change UI for the install queue to match bulk operations Active .
We can have discuss and come up with a solution so that all the styling issues can be solved using a finalised approach.

utkarsh_33 → created an issue.

I am unable to see a proper preview for the cards component.

attaching the ss for claro and gin after the fix.

utkarsh_33 → created an issue.

The main branch is fixed now. Hiding the other branch.
All the problems are fixed in Gin (light and drak mode) as well as Claro. This is up for another round of reviews.

utkarsh_33 → changed the visibility of the branch change-ui-for to hidden.

I think this issue was fixed in this commit..
I manually tested this in chrome and safari but i cannot reproduce the issue on the latest 2.0.x.
@rokoller can you again check if this issue still persists?If yes can you specify the browsers in which you can reproduce this issue?

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

Just to avoid confusions:-

1. change-ui-for branch has completely working code.
2. 3484474-change-ui-for branch has some issues which i had a hard time to figure out what went wrong while merging so i created a new branch to compare and show the working status.

Meanwhile i figure out what have i have messed up in the older branch someone reviewing the code can take the checkout of the new branch as it works as expected.
Thansks!

I added a fix that adds the classes to respective elements based on the themes.IMHO it can also be an approach to fix the issues rather that including a separate library via hooks(i might be wrong though).
We can still try to optimise the changes if the approach looks good.Marking it NR to get some feedbacks from @rkoller, @phenaproxima or someone else.

Can we do that in a follow-up as it might take some time and i think the remaining part of the issue is fixed and might be delayed due to that small regression.
What are your thought @rkoller?

the regular button is missing a background color and the button border same as the button label have with that dark blue against the black background a way too low contrast.

I have used the already existing class from drupal's codebase and that already has background color and border color defined.

Fixed the issue mentioned in #33.Ready for reviews.

Marking it NR as all the feedbacks in the scope of the issue are resolved.I think it would make sense to create a follow-up for what @rkoller mentioned in #42.

I have fixed the following issues according to the discussion in drupal slack:-

1. The Gin themes issue related to the buttons is now fixed(which is also mentioned in #41)
2. The sticky behaviour is restored by bringing back the views-bulk-actions class back as changing the classes according to the discussion in this makes a lot of copy pasting the classes which increase the overhead.
3. The reset button is now a secondary button.

Attaching the SS for reference how the buttons look in Gin theme:-

I have merged the latest changes from head.After reading comments from @phenaproxima regarding the visibility in this i realized that it makes sense to show the clear button only when any project is selected, so i made changes in this commit.If any one has any suggestion regarding this it would be good.

RE #9 Please see the recording.

RE #9:- I still see what i mentioned in #8.I'll try to record a video and upload.In the mean time if anyone else can also verify the current state.

At mobile, it seems to be still the single blue button - we need to also match mobile styles like we see on /admin/content, for example.

I just checked the issue, I was able to see both the buttons.Am i missing something?

Addressed all the remaining feedbacks.Ready for another round of reviews.

Addressed the latest feedbacks.Marking it NR again.

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

Rebased with latest changes form HEAD.Marking it NR again.

Merged the latest changes from 2.0.x.

Merged the latest changes from 2.0.x.

I have added a clear selection button.It's ready for another round of review.

Fixed the issue outlined in #18.Marking it NR.

Re #31:

about #28.3 there is one detail i've just realized when retesting. when you get to the end of a list by pressing arrow down and then immediately continuing on top and the other way around when you press the up arrow and when reaching the first option and then you immediately continue from the bottom only applies when voice over is enabled. with voice over disabled on select lists you dont jump to the other end of the list and continue, you just stop.

I am not able to reproduce this atleast in Chrome and Safari browser.The arrows work fine for me in both the cases.

and could you provide me some advice how to best switch between commits

I usually use git log to get the commit and checkout that commit id which i want to test.

I have verified that all the Drupal.t() usages are now correctly used and changes look good to me, so marking this RTBC.

I have addressed all the feedbacks on #28.Hope all the stuff on this are now done.@rkoller can you review this once again?

Re #28.1 I just tested it on safari browser but i was not able to reproduce what is mentioned.I am attaching the screen recording for the same demonstrating how it works on my local.

Added test as requested.If this is enough to cover the use case that we are fixing here then can we remove the needs test tag?

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

utkarsh_33 → created an issue.

While testing this MR i found one issue with the current state of the MR.
When we click on install(when max_selection is set to 1 by default) the other button's label on the module cards toggle to select .Attaching the SS for the same.

While the captcha module is being installed you can see the other module's cards have select as the button's label.

Merged the latest changes.

I tried to get the other tests pass but they are not working.

I followed the discussions on this issue and thought of trying to implement it the config ways.Attaching the patch(which is just a POC) for what i tried to disable the preview button for now.

Any one has any thoughts on this approach?

This is duplicate of The project title's line height is a bit off. 🐛 The project title's line height is a bit off. Active .We can close any one of them.

This is how it looks after the fix:-

utkarsh_33 → created an issue.

In this commit we can simply replace the role to menu and the sub div's role to menuitem if we want the multiselect filter to behave like the other filters but the problem is that it focuses on the whole div instead of the checkbox.I am pushing the changes with the above suggestions but someone testing this might switch to prior commit to see another version of this MR.

Re #23
With voiceover switched on now all the things mentioned are resolved except 5 which i am not sure whether we want to do that as the 2 elements are different in nature.
I have tested everything else on Safari and chrome and it works fine in these two browsers.It would be helpful if you can have a look at this once again @rkoller and verify the changes.
Marking it NR.

After reading through the issues that @rkoller listed in #32 i have. my own set of suggestions of how we can handle the issues as a seperate follow-ups for each of them.I'll describe them one by one.

1. The issue related to the
   

   if you have selected several entities across several pages of a source type

   it would be hard Or i would say not a good UI/UX to keep a track of which module was selected from which page as a user might change the number of module that should be listed on page at any point of time.I would suggest to keep a track of all the modules that we are going to install and the show a pop-up or maybe a simple UI that is shown in case of the uninstall modules page in Drupal core where we list the modules that are going to be un-installed.
   This might also solve the issue mentioned in
   

   n the context of an unexpected number of modules installs, would it be reasonable to add an option to the project browser settings to enable a confirmation step for the bulk install option? that the site builder gets an overview of the things being installed and has to confirm that first? that the person has the situational awareness what is happening next (also the case that an accidental selection of the wrong module was made?)

2. about the selection being persistent

   I think the better way to solve this problem of selection persitence is to give a reset button for now which simply deletes all the selections from a particular source type(or maybe all the source types at once) and then further we can provide an option to individually remove modules in case if a user has selected something by mistake.

I agree with @rkoller that variant B is a better approach to solve this fundamental issue that exists right now.

ps. in the drupal dojo austin where i'Ve presented the current state of project browser i got a few ideas/suggestions in regards of source types and filters that might apply here as well. but i havent had the time to summarize the findings. will catch up on this in the next few days.

Is there anything that you wanted to update or add according to the above comment or shall i start with the approach in variant B?

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

So currently if we apply a recipe the status card shows installed but as soon as we refresh the page the select button appears again.I am not sure if that's the desired behaviour or we want something like once the recipe is installed a re-install button should appear appear on the card(which i think is a better approach and a more user friendly approach).
Marking it NR to get others opinions on this.

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

Tried to wrap-up the remaining task on this.I am not sure whether i have covered all the stuff that was expected out of the remaining work.If not it would be great if someone can specifically point out the remaining stuff so that we are on the same page of what all needs to be added as a part of this issue.

Addressed the feedbacks.

Ready for review and manual testing.

This is ready for reviews now.The CI fails are unrelated.

This is ready for review.The CI(Eslint) is failing on all the branches so that can be ignored.Marking it NR.

@rkoller can you have a look at this again?I have addressed feedbacks that i think can be addressed as a part of this issue.

I have changed the deselect button to secondary button.

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

No you can take that if you want.

I am not sure if color of Install selected projects and Select/Unselect is correct or not.

So i was trying to make this inline with the comment mentioned in #5 📌 Change UI for the install queue to match bulk operations Active by @chrisfromredfin. Not sure if that's not what we want.It would be helpful if someone clearly describes what behaviour we want for the select and deselect buttons.
Or i think we can take that as a follow-up because this issue is majorly fixing the install button behaviour to match the bulk action.

Addressed all the feedbacks and tests are also passing.Ready for review.

The tests are failing.Can you rerun the CI job, as they might be random fails.

Not sure whether there is a better way of doing this but now the focus is on the modal element.Marking it NR.

I have created a follow-up as requested Remove any existing reference to queue/dequeue from Project browser code 📌 Remove any existing reference to queue/dequeue from Project browser code Active .

utkarsh_33 → created an issue.

I'll try to work on this.

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

I tried adding the fix

$this->getSession()->evaluateScript('document.getElementById("67").scrollIntoView()');

,but it's not working for all the tests.

I have implemented one of the two suggestions.

I think we should add tests for this as well.Will work on writing tests.

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

This is the latest look of the card.

Resolved all the open threads and addressed feedbacks as well.Marking it NR again.

Added tests and also addressed feedbacks.

I have addressed the feedbacks regarding the color contrast.I am not sure about which checkmark icon is @rkoller talking about as this was not changed in this MR any where, it is what it was on 2.0.x.
Also the cursor: not-allowed was already added in prior commits.
@rkoller can you now verify that the changes?

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

Updated with the latest changes.Marking it NR.

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

The issue reported is fixed and now we can see the images.Marking it NR.

I think we shall consider Adjust the micro copy for the Recipes source type to the Recipes terminology 📌 Change the button label for Recipes from install to apply Active as well.If we have a single queue across all the tabs but different terminologies across tabs then it would create more confusions.
Once Adjust the micro copy for the Recipes source type to the Recipes terminology 📌 Change the button label for Recipes from install to apply Active gets in then we might have button text as apply for the recipes tabs and install across different module tabs which might be confusing.
Just an opinion 😅.

This is ready for reviews as the tests are all green now.The current state of the MR is inline with what was suggested in #11 ✨ Filter by category should be scrollable Active .

The only thing that's not happening correctly is when i select say Z-A sort type in Random-data plugin and i visit some source plugin which does not have that sort option in the select list (Contrib modules in this case) then the sort option is set to default across all the tabs.Not sure whether we want this or something else.
Marking it NR for getting some opinions on what we want.

So i have recorded a video demonstrating what's happening on 2.0.x.In this video i have 4 sources enabled.