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

Chore/add test for editor #508

Open
wants to merge 19 commits into
base: feature/a11ymap
Choose a base branch
from

Conversation

Mayaryin
Copy link
Contributor

@Mayaryin Mayaryin commented Mar 11, 2025

🖊️ Description

Added tests for the editor components WheelchairEditor, ToiletsWheelchairEditor and StringFieldEditor.
The StringFieldEditor is used in two cases: for adding a new description and for editing the currently displayed description.
The feature details are mocked for testing.
Two test cases are still missing: keyboard navigation and wcag compliance. These will be addressed in another issue, since especially wcag compliance needs some refactoring of the components. This PR is to get some tests out for this feature.
I also moved the Editor components to a new modules directory called "edit" and also placed the tests there.

GIF

📎 Related Ticket

⚠️ Creation checklist

Before creating this merge request, go over the following checklist (click to expand).
Remove any items that are not applicable.

💪 I have tested my code
  • A new E2e playwright test covers this feature
  • The feature deployment works.
  • The automated tests are passing.
🧼 I have cleaned up my code
  • I have removed debug logging.
  • My code does not depend on new vulnerable packages.
  • The commit messages are precise and make sense (rebase the PR with --interactive if applicable, keeping commits in sensible chunks if possible).
🔍 I have performed a self-review of my code
  • My code is self-documenting or has links to necessary documentation.
  • New function and variables names can be understood by new developers with basic project knowledge.
✨ I have created a nice pull request
  • It has a clear title.
  • It follows the template, has a clear description and testing instructions if needed.
  • It references applicable Asana tickets.
  • It targets the right branch.
  • I removed not applicable sections of the PR template.
  • [optional] I added a GIF of my favorite animal to the PR description to lighten the mood of my colleagues.

🔍 Reviewing

When reviewing this merge request, here are some things to keep in mind:

@Mayaryin Mayaryin marked this pull request as ready for review March 11, 2025 17:06
Mayaryin added 19 commits March 11, 2025 18:06
- added a picker to  StringFieldEditor.tsx
- made StringFieldEditor.tsx and AutoEditor.tsx aware of the selected picker value
- added util function to strip off any language tag if present
- added new language tag to osm tag if the language is -changed

ToDos:
- move icons to another location/inside the picker file
- get and organize a list of ietf language tags to display in the drop down
- make button unclickable if no language is selected?
- handle the case that someone wants to add a new localized description when a general description already exists --> what should happen to the text in the text area if the language is changed?
@Mayaryin Mayaryin force-pushed the chore/add-test-for-editor branch from 62e41d1 to 6988dbe Compare March 11, 2025 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant