Use keyboard to select values

[jensvannerum]

References

• Fixes Can't use keyboard to select value in submission form dropdown #2500
• Fixes Submission dropdowns do not load previously selected and saved value after reload #2805

Description

These changes allow for keyboard navigation in submission dropdowns

Instructions for Reviewers

This PR modifies the DsDynamicScrollableDropdownComponent to support keyboard navigation. This is done by calling the already existing getVocabularyEntriesByValue instead of just calling getVocabularyEntries like done before.
Some helper methods where needed to handle the user input as we don't want to allow all characters. A method for handling user input already existed (selectOnKeyDown) so this was pretty straight forward.

Also moved the code from the ngOnInit method away to a reusable loadOptions method.

how to test this PR
This can be tested against the demo instance (demo.dspace.org) using the "type" dropdown in submission.

However I would recommend to test this locally and add a lot of values to the dropdown starting with the same letter(s) so you can verify that the scrolling / pagination functionality also works when filtering results.

Checklist

• My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
• My PR passes ESLint validation using yarn lint
• My PR doesn't introduce circular dependencies (verified via yarn check-circ-deps)
• My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
• My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
• If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
• If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
• If my PR fixes an issue ticket, I've linked them together.

[alanorth]

Thank you, @jensvannerum. This allows the list to be filtered with the keyboard, but it is still not possible to select the value without using the mouse to click it. Our editors are very used to doing this from the DSpace 6 XMLUI submission form and it's important for accessibility as well. Do you think it would be possible to implement?

Also, I noticed that the matching occurs on the stored values, but I'm wondering if we are hitting a similar issue to #2724. Here I have a vocabulary with SPDX licenses. The displayed values are human readable, but the stored values are like "CC-BY-4.0":

I can type "CC-", but as soon as I type "CC-B" the filtered list goes blank. Similary, I can type "4." and the list filters, but when I type "4.0" the list goes blank.

---

For reference, the configuration for this vocabulary is like:

       <pair>
         <displayed-value>Creative Commons Attribution 4.0 (CC BY 4.0)</displayed-value>
         <stored-value>CC-BY-4.0</stored-value>
       </pair>

[jensvannerum]

Thanks for your feedback @alanorth, it indeed seems like the filter is being done on the value of the vocabulary, not the label, this is a separate issue in the back-end that I fixed and will make a PR linked to this one for.

As far as the keyboard selection goes, how do you visualise this ?

It is still possible to enter the list of filtered values by pressing Tab like before and navigating with the arrow keys and enter. I am assuming you would like to be able to instantly press enter but this would only be possible when only 1 value is remaining in the filtered list.

[alanorth]

the filter is being done on the value of the vocabulary, not the label, this is a separate issue in the back-end that I fixed and will make a PR linked to this one for.

It's out of the scope of this PR I think, but good to know. For some vocabularies the display value and the stored value are the same, but not always. If it is possible to search both the display and the stored values that would be nice.

As far as the keyboard selection goes, how do you visualise this?

I think it should work as close to a normal HTML input as possible. Try for yourself in GitHub's search bar, google.com, or even a simple HTML form input element. If you type a few characters and press the down arrow key you can select values without needing the mouse.

Thank you!

[jensvannerum]

@alanorth I've updated the PR with keyboard navigation that matches the native html functionality as closely as possible

[alanorth]

Thank you @jensvannerum. I'm having problems applying the last commit in this patch series:

110088: changes to keyboard navigation / selection

I'm not exactly sure why or how, but the state of your src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.html seems different than the one on main. Can you rebase your branch?

[alanorth]

Thanks again @jensvannerum. I re-based your branch on top of latest main locally and tested it in combination with DSpace/DSpace#9276 and the experience is working well—much more like a native HTML form element.

I'm not sure why I had issues with the branch. I have rebased (and squashed your four commits into one) on top of the latest main and merged manually: c02b46c. I will port it to dspace-7_x as well.

[tdonohue]

NOTE: Despite this looking like it was simply "closed", this PR was merged manually via c02b46c (as noted in previous comment).

This confused me at first, so I wanted to call it out as the PR status of "Closed" implies that it wasn't merged.

[alanorth]

@tdonohue yes please accept my apologies! I was having git conflicts when trying to merge this PR's branch to main locally and was surprised that GitHub's UI didn't show a conflict as well. Also, because I saw there were four commits all working towards the final working solution, including one "oops, fix linting" commit, I decided to manually rebase on top of main and squash the commits into one to be more atomic (preserving authorship for @jensvannerum of course).

In a way I'm inspired by Linus Torvald's management of the Linux kernel. You can do whatever dirty things you want in your feature branches, but the time your pull request arrives on the master branch it should be a clean, atomic commit. Our git history will be here long after GitHub is gone!

[tdonohue]

@alanorth : Just a note for the future. There is a merge option in GitHub which does a "squash". Take a closer look at the green merge button on a PR. It has a dropdown which offers additional merge options including:

• Create a merge commit (default - merge all the PRs commits via a merge commit)
• Squash & merge (entire PR squashed into a single commit and then merged)
• Rebase & merge (rebase first, then merge all the PRs commits)

I think you might have been able to use "Squash & merge" in this scenario. You'd have to do the squash & merge again though on the auto-ported PR (as the ported PR will include all the commits again).

[alanorth]

@tdonohue Thanks. Yes I know about the other options for merging on GitHub. The problem is that you can rebase or you can squash—but not both! It's very easy to do via the git command line. I considered doing the work manually and then force pushing to the branch, but the submitter did not check the box allowing maintainers to update their branch. And seeing as this was a 1 APPROVAL I figured it wasn't too big of a deal.

Only down side is that we have a red "Closed" badge on GitHub...

[alanorth]

Oops, it seems this introduced a new bug where dropdown values are cleared when moving between submission steps. See:

Screencast.from.2024-01-30.11-38-39.webm

@jensvannerum would you be able to take a look at this? Hopefully something minor in the implementation. Thank you!

[jensvannerum]

Re-opened this request since this introduced a new bug which I fixed now in context of #2805

[tdonohue]

@alanorth or @toniprieto : Could one of you test this to verify it now fixes #2805 ?

(@jensvannerum : Just a sidenote... in the future we may want to open a separate PR instead of reopening the same PR. This entire PR has gotten confusing as your prior fixes applied to main in c02b46c are no longer visible in this PR. Now the PR only shows your brand new fixes. This makes it harder for others to understand the changes that were necessary to fix this issue. For now though, we can go with this approach. Just wanted to point out that we should always try to create a new PR once a PR is closed/merged.)

[toniprieto]

Thanks @jensvannerum , it's solve the issue, though I would make some additional changes to make it more similar to the original code. Comments below:

[alanorth]

Thanks @jensvannerum. I tested this patch by cherry-picking the two latest commits into my 7.6 branch, then starting a new item submission. The drop downs are filterable with the keyboard and selected values are kept as expected when moving between submission sections. I was able to deposit the item successfully and all metadata was preserved.

@tdonohue I'm not sure what we want to do about the commits here, can we merge a pull request twice on GitHub? It certainly seems so in the UI...

[toniprieto]

Thanks @jensvannerum !

[dspace-bot]

Backport failed for dspace-7_x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin dspace-7_x
git worktree add -d .worktree/backport-2733-to-dspace-7_x origin/dspace-7_x
cd .worktree/backport-2733-to-dspace-7_x
git switch --create backport-2733-to-dspace-7_x
git cherry-pick -x 4bbfb4cc1a5b4e7cadf6b2e9be1d89f9873901cd 74f1c613f66c25a5af0fbd1ccc476baa4290efdc 3a265bb0e7c6743a007a666abedad1f6695e48b0 05eb21250efcbe94fbb174d35bf170663a0d8b13 9767260b9ab3d9b7db56c57d494f6619c31165ce f7f86b0a9e493484c48de53bffa982a795443a75

[tdonohue]

NOTE: This PR was merged in several stages:

• Part 1 of PR (first 5 commits)
  • Manually merged/squashed to main in c02b46c
  • Manually merged/squashed to dspace-7_x in [Port dspace-7_x] Use keyboard to select values #2764
• Part 2 of PR (last 2 commits)
  • Merged to main via merging this PR
  • Manually merged to dspace-7_x via [Port dspace-7_x] Use keyboard to select values (part 2) #2821