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

[stable27] fix: checkbox now allows shift-click and multi-select #43556

Conversation

emoral435
Copy link
Contributor

@emoral435 emoral435 commented Feb 13, 2024

Summary

PROBLEM: Shift clicking the files in the files app of stabl27 does not allow the functionality to select multiple files at once

  • Originally, I thought that this was regression within the Vue #app-content components, but after testing, I found that it was due to the logic behind our legacy PHP templates and the JS that was bundled with it
  • To fix this, shift the event to the general select class. That means that the onChange behavior is not needed - we can now use onClick and prevent the default action that causes double clicking, as we do the clicking functionality ourselves
  • The check used to see if we are choosing a range did not fall in line with the other shift-select logic, so I made them now similar
🏚️ Before 🏡 After
https://github.com/nextcloud/server/assets/110193237/cb714e8f-ac41-484c-bf88-7351612c7286 https://github.com/nextcloud/server/assets/110193237/db975ecc-e2c4-433c-a6a9-e389c47469d2

PS.. sorry for the weird links, but the gifs showing the difference are there^

Checklist

@emoral435 emoral435 added this to the Nextcloud 27.1.8 milestone Feb 13, 2024
@emoral435 emoral435 self-assigned this Feb 13, 2024
@emoral435 emoral435 linked an issue Feb 14, 2024 that may be closed by this pull request
@skjnldsv skjnldsv changed the title fix: checkbox now allows shift-click and multi-select [stable26] fix: checkbox now allows shift-click and multi-select Feb 14, 2024
@skjnldsv skjnldsv changed the title [stable26] fix: checkbox now allows shift-click and multi-select [stable27] fix: checkbox now allows shift-click and multi-select Feb 14, 2024
@solracsf solracsf force-pushed the fix/42364/allow-multi-select-for-files-when-clicking-checkbox branch from 3022b23 to 962905f Compare February 15, 2024 12:54
@emoral435 emoral435 force-pushed the fix/42364/allow-multi-select-for-files-when-clicking-checkbox branch 2 times, most recently from e86de73 to 9771a8e Compare February 19, 2024 18:30
@skjnldsv skjnldsv mentioned this pull request Feb 20, 2024
9 tasks
@emoral435 emoral435 force-pushed the fix/42364/allow-multi-select-for-files-when-clicking-checkbox branch from 9771a8e to ec5e7b1 Compare February 20, 2024 13:03
@skjnldsv skjnldsv mentioned this pull request Feb 21, 2024
@solracsf solracsf added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Feb 21, 2024
@skjnldsv skjnldsv force-pushed the fix/42364/allow-multi-select-for-files-when-clicking-checkbox branch from ec5e7b1 to f5516f2 Compare February 21, 2024 17:35
@skjnldsv
Copy link
Member

files.cy.ts is failing

@skjnldsv skjnldsv added 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish labels Feb 22, 2024
@emoral435
Copy link
Contributor Author

emoral435 commented Feb 22, 2024

@skjnldsv Cypress failures seem unrelated: re-ran tests again and they passed 🎉 (failed test is the blocker). Proof:
image

Still needs to be rebased, but should be ready to be merged 🎉 Question though, I tried running npm run cypress:e2e but it gets stuck here ->
image
Any ideas why? This is related to another PR I want to get merged in for stable28

@emoral435 emoral435 force-pushed the fix/42364/allow-multi-select-for-files-when-clicking-checkbox branch from f5516f2 to abcbea3 Compare February 22, 2024 15:25
@solracsf
Copy link
Member

Cypress tests seems to be flaky since 2 days in various PRs 😩

@skjnldsv
Copy link
Member

Yeah, we notived some missing cypress fixes for 27.
Master is getting some other fixes too

@skjnldsv skjnldsv merged commit fc4b5fc into stable27 Feb 22, 2024
35 of 37 checks passed
@skjnldsv skjnldsv deleted the fix/42364/allow-multi-select-for-files-when-clicking-checkbox branch February 22, 2024 18:31
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish bug feature: files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Allow multi-select for files in trash
3 participants