Skip to content

Use "variant name [variant tag]" format everywhere (review, write, profile) #4865

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

Merged

Conversation

HarikalarKutusu
Copy link
Contributor

@HarikalarKutusu HarikalarKutusu commented Apr 9, 2025

This PR fixes/implements:

@moz-dfeller & @moz-rotimib :

  • This might need a bit more refactoring afterwards. I think the variant display interface can be made better (e.g. with hooks or passing the Variant[] as a whole), so that it can be included from a single place. Waiting for comments.
  • In WRITE, when selection completed, only the tag is shown in the form field. The Select component interface is a bit limiting, but can be fixed in refactoring.

image

Large screen with long textual parts test (seems OK):
image

Same on smaller screens (seems OK):
image

Profile:
image

Write:
image

@HarikalarKutusu HarikalarKutusu requested a review from a team as a code owner April 9, 2025 20:44
@HarikalarKutusu HarikalarKutusu requested review from moz-dfeller and removed request for a team April 9, 2025 20:44
@HarikalarKutusu HarikalarKutusu changed the title [WIP] Add variant names to the pipeline to present to the user in review page Add variant names to the pipeline to present to the user in review page Apr 9, 2025
@HarikalarKutusu HarikalarKutusu changed the title Add variant names to the pipeline to present to the user in review page Use "variant name [variant tag]" format everywhere (review, write, profile) Apr 13, 2025
Copy link
Contributor

@moz-dfeller moz-dfeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @HarikalarKutusu, looks good for me from the backend perspective. There are FE tests that are failing though and have to be fixed and one merge conflict (due to the recent random sentences update).

@HarikalarKutusu
Copy link
Contributor Author

HarikalarKutusu commented Apr 14, 2025

Missed one test :( yarn test gives too much output...

@moz-rotimib, any comment/request is very much welcome.

Copy link
Contributor

@moz-rotimib moz-rotimib left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@HarikalarKutusu HarikalarKutusu self-assigned this Apr 15, 2025
@HarikalarKutusu HarikalarKutusu added Enhancement A idea to enhance and existing feature or process on Common Voice Frontend Relates to frontend code labels Apr 15, 2025
@moz-dfeller moz-dfeller merged commit d40f8df into common-voice:main Apr 17, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement A idea to enhance and existing feature or process on Common Voice Frontend Relates to frontend code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants