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

test: Extend getNextInputElement tests for several elements per cell #14160

Merged
merged 3 commits into from
Nov 27, 2024

Conversation

TomasEng
Copy link
Contributor

@TomasEng TomasEng commented Nov 25, 2024

Description

When adding the text resource picker to the input table, the table will have cells with several interactive elements (one input field and two toggle buttons). In this case, when the user navigates using the arrow keys, it is the text field (or the combobox when in search mode) that should receive focus. This may be generalized to the first interactive element in the cell. Therefore, I have added tests that verify that this is the case. There is no need to change the actual function since it turns out that the new tests run green without modifying anything.

Note

It may be a good idea to evaluate this behaviour with some user testing. By implementing this, we are implying that arrow keys should be used to always navigate from one cell to another, and not inside the cell itself. This is also the most straightforward and simple rule to support. Implementing other kinds of behaviour might be complicated because of design system limitations, element positioning and so on.

Related Issue(s)

Verification

  • Your code builds clean without any errors or warnings

@github-actions github-actions bot added solution/studio/designer Issues related to the Altinn Studio Designer solution. frontend labels Nov 25, 2024
@TomasEng TomasEng marked this pull request as ready for review November 25, 2024 19:48
Copy link

codecov bot commented Nov 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.36%. Comparing base (cd8560b) to head (fccc454).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #14160   +/-   ##
=======================================
  Coverage   95.36%   95.36%           
=======================================
  Files        1797     1797           
  Lines       23367    23367           
  Branches     2702     2702           
=======================================
  Hits        22284    22284           
  Misses        836      836           
  Partials      247      247           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@standeren standeren self-assigned this Nov 26, 2024
@standeren standeren assigned TomasEng and unassigned standeren Nov 26, 2024
@TomasEng TomasEng added skip-releasenotes Issues that do not make sense to list in our release notes skip-manual-testing PRs that do not need to be tested manually skip-documentation Issues where updating documentation is not relevant labels Nov 27, 2024
@TomasEng TomasEng merged commit 707fe2c into main Nov 27, 2024
13 checks passed
@TomasEng TomasEng deleted the extend-get-next-input-element-tests branch November 27, 2024 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend skip-documentation Issues where updating documentation is not relevant skip-manual-testing PRs that do not need to be tested manually skip-releasenotes Issues that do not make sense to list in our release notes solution/studio/designer Issues related to the Altinn Studio Designer solution. team/studio-domain1
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants