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

fix(FilePicker): Fix selecting rows using the keyboard #934

Merged
merged 3 commits into from Aug 25, 2023

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Aug 23, 2023

Selecting by keyboard was broken due to wrong event and key name.

@susnux susnux added bug Something isn't working backport-request 3. to review labels Aug 23, 2023
@susnux susnux requested a review from skjnldsv August 23, 2023 23:30
@susnux
Copy link
Contributor Author

susnux commented Aug 24, 2023

@susnux
Copy link
Contributor Author

susnux commented Aug 24, 2023

Not sure what to do here. Merge without tests? Is there some way to ignore the package exports? Last resort: Fork the project as like with vue-select?

@skjnldsv
Copy link
Contributor

skjnldsv commented Aug 24, 2023

Not sure what to do here. Merge without tests? Is there some way to ignore the package exports? Last resort: Fork the project as like with vue-select?

Import with relative path.

import xxx from '../../node_modules/webdav/dist/'

@susnux
Copy link
Contributor Author

susnux commented Aug 24, 2023

Import with relative path.

This might work for apps or server, but not for libraries. You can not know that the package is installed at that location.
So for libraries this only works if we bundle the dependency inside the library.

As all of those are possible:

server
  |- node_modules
      |- @nextcloud/dialogs
      |- webdav

or

server
  |- node_modules
      |- @nextcloud/dialogs
          |- node_modules
              |- webdav

or

server
  |- node_modules
      |- @nextcloud/dialogs
          |- node_modules
              |- @nextcloud/files
                  |- node_modules
                      |- webdav

@skjnldsv
Copy link
Contributor

Indeed, I only tried with Apps

@susnux
Copy link
Contributor Author

susnux commented Aug 25, 2023

See #949 for a solution until it is fixed, after that one is merged, this can be rebased and should work :)

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
…d (except for entering directories)

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux merged commit d73abda into master Aug 25, 2023
8 checks passed
@susnux susnux deleted the fix/keyboard-selecting branch August 25, 2023 10:51
@skjnldsv skjnldsv mentioned this pull request Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants