Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[full-ci] Only show batch actions for more than one selected resource #6351

Closed
wants to merge 2 commits into from

Conversation

pascalwengerter
Copy link
Contributor

@pascalwengerter pascalwengerter commented Feb 1, 2022

Description

Kinda experimental
Note: AppBar/SizeInfo/BatchAction unit tests need a major overhaul before this can get merged
now that #6662 got merged this can be easily continued

Related Issue

  • Fixes to_be_defined

Motivation and Context

Reduce "jumpiness" of UI, only show batch actions above files table when relevant

TO-DOs

  • Refactor AppBar/SizeInfo/BatchAction unit tests
  • Green E2E & Acceptance tests
  • Changelog item

@update-docs
Copy link

update-docs bot commented Feb 1, 2022

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@ownclouders
Copy link
Contributor

Results for e2e-tests oCIS https://drone.owncloud.com/owncloud/web/22339/11/1

💥 To see the trace, please open the link in the console ...

npx playwright show-trace https://cache.owncloud.com/owncloud/web/tracing/22339/2022-2-1-08-15-40-Alice.zip

npx playwright show-trace https://cache.owncloud.com/owncloud/web/tracing/22339/2022-2-1-08-15-46-Brian.zip

@sonarcloud
Copy link

sonarcloud bot commented Feb 1, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug B 1 Bug
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

90.0% 90.0% Coverage
0.0% 0.0% Duplication

@ownclouders
Copy link
Contributor

Results for oC10SharingAndTrashbin https://drone.owncloud.com/owncloud/web/22340/32/1

💥 The acceptance tests failed on retry. Please find the screenshots inside ...

webUITrashbinDelete-trashbinDelete_feature-L99.png

webUITrashbinDelete-trashbinDelete_feature-L99.png

@ownclouders
Copy link
Contributor

Results for oC10Move https://drone.owncloud.com/owncloud/web/22340/19/1

💥 The acceptance tests failed on retry. Please find the screenshots inside ...

webUIMoveFilesFolders-moveFiles_feature-L139.png

webUIMoveFilesFolders-moveFiles_feature-L139.png

@ownclouders
Copy link
Contributor

Results for oCISTrashbinUploadMoveJourney https://drone.owncloud.com/owncloud/web/22340/64/1

💥 The acceptance tests failed on retry. Please find the screenshots inside ...

webUITrashbinDelete-trashbinDelete_feature-L99.png

webUITrashbinDelete-trashbinDelete_feature-L99.png

@ownclouders
Copy link
Contributor

Results for oC10Files5 https://drone.owncloud.com/owncloud/web/22340/18/1

💥 The acceptance tests failed on retry. Please find the screenshots inside ...

webUIFiles-batchAction_feature-L113.png

webUIFiles-batchAction_feature-L113.png

webUIFiles-batchAction_feature-L14.png

webUIFiles-batchAction_feature-L14.png

webUIFiles-batchAction_feature-L24.png

webUIFiles-batchAction_feature-L24.png

webUIFiles-batchAction_feature-L62.png

webUIFiles-batchAction_feature-L62.png

webUIFiles-batchAction_feature-L75.png

webUIFiles-batchAction_feature-L75.png

@ownclouders
Copy link
Contributor

Results for oCISSharingInternal2 https://drone.owncloud.com/owncloud/web/22340/56/1
The following scenarios passed on retry:

  • webUISharingInternalUsers/shareWithUsers.feature:342

@ownclouders
Copy link
Contributor

Results for oCISFiles1 https://drone.owncloud.com/owncloud/web/22340/53/1

💥 The acceptance tests failed on retry. Please find the screenshots inside ...

webUIFiles-batchAction_feature-L113.png

webUIFiles-batchAction_feature-L113.png

webUIFiles-batchAction_feature-L14.png

webUIFiles-batchAction_feature-L14.png

webUIFiles-batchAction_feature-L24.png

webUIFiles-batchAction_feature-L24.png

webUIFiles-batchAction_feature-L62.png

webUIFiles-batchAction_feature-L62.png

webUIFiles-batchAction_feature-L75.png

webUIFiles-batchAction_feature-L75.png

@pascalwengerter
Copy link
Contributor Author

Needs a rebase once #6358 is merged but should be quite easy to finalize ✊🏽

@pascalwengerter
Copy link
Contributor Author

Needs a rebase, I'll take care of it today

@pascalwengerter pascalwengerter force-pushed the batchactions-select-min-two branch 3 times, most recently from 1bc21f3 to b7c3585 Compare March 30, 2022 10:59
@tbsbdr
Copy link
Contributor

tbsbdr commented Mar 31, 2022

reduces unnecessary changes in the ui - feels really good

NEW - what you see in the video: batch actions are only shown, if at least 2 items are selected. i.e. if only 1 item is selected, batch actions are not shown (batch of 1 items doesnt make sense imo)

2022-03-31_15-38-49 (2)

@pascalwengerter pascalwengerter force-pushed the batchactions-select-min-two branch from b7c3585 to 8078e80 Compare March 31, 2022 20:01
@sonarcloud
Copy link

sonarcloud bot commented Mar 31, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

87.5% 87.5% Coverage
0.0% 0.0% Duplication

@ownclouders
Copy link
Contributor

ownclouders commented Mar 31, 2022

Results for e2e-tests oCIS https://drone.owncloud.com/owncloud/web/26626/12/1

💥 To see the trace, please open the link in the console ...

npx playwright show-trace https://cache.owncloud.com/public/owncloud/web/26626/tracing/unstructured-collection-of-testable-space-interactions-alice-2022-7-4-03-59-06.zip

@tbsbdr
Copy link
Contributor

tbsbdr commented Apr 4, 2022

received positive feedback on the changes :-) lets do it.
@elizavetaRa noted that the gaps between the buttons could be bigger to get a better impression of what icon belongs to which text 👍

would be nice, if the Empty trashbin button would not change to raw

Current:
image

proposal with gaps aligned:
image

fyi @ChrisEdS

@settings settings bot removed the ux-overhaul label Apr 6, 2022
@pascalwengerter pascalwengerter force-pushed the batchactions-select-min-two branch from 8078e80 to 5d45b7d Compare June 15, 2022 16:05
Copy link
Contributor

@tbsbdr tbsbdr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pascalwengerter could you align the gaps of the row as mentioned in
#6351 (comment)
to be more precise:
Clipboard - June 15, 2022 7_06 PM

everything else is fine, thanks!

@pascalwengerter pascalwengerter force-pushed the batchactions-select-min-two branch from 5d45b7d to 9b6fbe9 Compare July 4, 2022 15:42
@sonarcloud
Copy link

sonarcloud bot commented Jul 4, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

76.7% 76.7% Coverage
0.0% 0.0% Duplication

@JammingBen
Copy link
Contributor

@tbsbdr Whats the state here? The batch actions changed meanwhile - they are outlined buttons now, I think that's fine. What about the change to only show batch actions if >1 files are selected?

@kulmann
Copy link
Member

kulmann commented Oct 27, 2022

@tbsbdr Whats the state here? The batch actions changed meanwhile - they are outlined buttons now, I think that's fine. What about the change to only show batch actions if >1 files are selected?

I personally really like the UX when only showing batch actions if >1 files are selected. I think we had a reason against it, but I don't remember. Do you @tbsbdr ?

@tbsbdr
Copy link
Contributor

tbsbdr commented Oct 28, 2022

don't remember

sry, me neither.

jep, should still go with the "Only show batch actions for more than one selected resource" approach.

@pascalwengerter
Copy link
Contributor Author

I think I just never followed on with this PR fearing the huge pains when having to fix acceptance tests 🥴 hope someone else can pick up my changes and finish them for good?

@JammingBen
Copy link
Contributor

This has been superseded by #7904. Tests were indeed unpleasant to fix :D

@JammingBen JammingBen closed this Nov 3, 2022
@kulmann kulmann deleted the batchactions-select-min-two branch September 5, 2024 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants