Skip to content

made language selection understandable #822

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jan 6, 2024
Merged

made language selection understandable #822

merged 2 commits into from
Jan 6, 2024

Conversation

raghav1030
Copy link
Collaborator

@raghav1030 raghav1030 commented Jan 5, 2024 •
edited by jgadsden
Loading

Summary:

This PR changes the way languages are getting rendered. Initially the languages in the locale select dialog box was getting rendered by 3 letter ISO codes but now they have been locally transliterated into the respective language.

The update fixes a part of all the issues mentioned in #806. This PR changes the way languages are getting rendered.

Description for the changelog:
language selection dropdown menu uses natural language

Other info:
closes #806

@jgadsden jgadsden added enhancement New feature or request version-2.1 labels Jan 5, 2024
@jgadsden jgadsden modified the milestone: Version 2.2 Jan 5, 2024
@jgadsden
Copy link
Collaborator

jgadsden commented Jan 5, 2024

Superb to see this , many thanks @raghav1030
There are some unit tests failing because of the change in text, would you like to apply these changes and the tests should now pass :

diff --git a/td.vue/tests/unit/components/localeSelect.spec.js b/td.vue/tests/unit/components/localeSelect.spec.js
index d8fffd34..3a17bbdb 100644
--- a/td.vue/tests/unit/components/localeSelect.spec.js
+++ b/td.vue/tests/unit/components/localeSelect.spec.js
@@ -47,13 +47,13 @@ describe('components/LocaleSelect.vue', () => {
 
         it('has an option for eng', () => {
             expect(wrapper.findAllComponents(BDropdownItem)
-                .filter((c) => c.text() === 'eng').exists()
+                .filter((c) => c.text() === 'English').exists()
             ).toEqual(true);
         });
 
         it('has an option for deu', () => {
             expect(wrapper.findAllComponents(BDropdownItem)
-                .filter((c) => c.text() === 'deu').exists()
+                .filter((c) => c.text() === 'Deutsch').exists()
             ).toEqual(true);
         });
 
@@ -64,7 +64,7 @@ describe('components/LocaleSelect.vue', () => {
 
             it('updates the locale to deu', async () => {
                 await wrapper.findAllComponents(BDropdownItem)
-                    .filter(c => c.text() === 'deu')
+                    .filter(c => c.text() === 'Deutsch')
                     .at(0)
                     .trigger('click');
                 
@@ -73,7 +73,7 @@ describe('components/LocaleSelect.vue', () => {
 
             it('updates the locale to eng', async () => {
                 await wrapper.findAllComponents(BDropdownItem)
-                    .filter(c => c.text() === 'eng')
+                    .filter(c => c.text() === 'English')
                     .at(0)
                     .trigger('click');

@jgadsden jgadsden self-requested a review January 5, 2024 18:51
jgadsden
jgadsden approved these changes Jan 5, 2024
View reviewed changes
Copy link
Collaborator

@jgadsden jgadsden left a comment

Choose a reason for hiding this comment

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

good to see this change - tested and works as described
some unit tests now fail due to changes in text, but that is easy to fix up

@raghav1030
Copy link
Collaborator Author

Hey @jgadsden! I have updated the tests for locale select and now it is passing all the tests. Would love to know if there's any other feedback?

jgadsden
jgadsden approved these changes Jan 6, 2024
View reviewed changes
Copy link
Collaborator

@jgadsden jgadsden left a comment

Choose a reason for hiding this comment

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

Thanks for doing this @raghav1030 , all good to merge now

@jgadsden
Copy link
Collaborator

jgadsden commented Jan 6, 2024 •
edited
Loading

Going ahead with merge, the remaining failing test is is the usual 'lack of permissions' failure

@jgadsden jgadsden merged commit cade020 into OWASP:main Jan 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jgadsden jgadsden jgadsden approved these changes

Assignees

@raghav1030 raghav1030

Labels
enhancement New feature or request version-2.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

irritating things with user interface
2 participants
@raghav1030 @jgadsden