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

Translation Utility sorting inconsistencies #332

Closed
Tracked by #878 ...
Nancy-Salpepi opened this issue Dec 20, 2022 · 8 comments
Closed
Tracked by #878 ...

Translation Utility sorting inconsistencies #332

Nancy-Salpepi opened this issue Dec 20, 2022 · 8 comments

Comments

@Nancy-Salpepi
Copy link

Test device
MacBook Air (m1 chip) and Dell

Operating System
MacOS 13 and Win10

Browser
Safari and Chrome

Problem description
For phetsims/qa#869, if the sims are listed in ascending order and then I sort the sim-specific or common strings columns, sims with the same %age will be in alphabetic order. Pressing sort again so that the %ages go from least-->most, will result in sims with the same %age being in reverse alphabetic order.

Steps to reproduce

  1. Choose Portugese
  2. Scroll down to Sims translated into Portugese --sims are in alphabetical order
  3. Press the Sort button next to Common Strings --sims of the same % will still be in alphabetical order
  4. Press the Sort button again next to Common Strings --sims of the same % will be in reverse alphabetical order

Visuals

sortbuttons.mp4
@liammulh liammulh transferred this issue from phetsims/babel Dec 20, 2022
@liammulh
Copy link
Member

@oliver-phet, what are your thoughts on this? I think it's more of a design question. How should the sorting work ideally? Then once I have the "ideal" behavior, we can talk about engineering.

@oliver-phet
Copy link
Contributor

I get the same behavior in my testing. It would be nice if it kept Sim Title (ascending A-Z) - so the behavior is to preserve the existing sorting of the table but prioritize the new sort.

Essentially, I'm thinking:
Default logic: Sort by Sim Title (asc), then by Sim Strings % (asc), then by Common Strings % (asc)
Upon clicking a sort: that column is moved to the first priority of the logic (with opposite order).

If that is tricky/time consuming. Let's just sort by one column at a time, leave the others (asc).

Another peculiarity (that I think may be more disrupting to translators) is the inconsistent position of 'pH Scale'. If I sort by Sim Title, it is at the top or bottom (seems to be sorting uppercase/lowercase) but if I then sort by Sim/Common Strings, it is sorted correctly?

@oliver-phet
Copy link
Contributor

Oh - I also wanted to add Sim Strings % and Common Strings % are only sorting the %. When I plug this data into other data sorting tools (excel, google sheets) the entire string is sorted:
image

This also seems nice (allows translators to see among sims that have the same % which contain the most strings) and addresses some of @DianaTavares 's feedback in phetsims/qa#869 (comment).

@liammulh
Copy link
Member

@jbphet and I discussed this yesterday, and we think there might be some issues with sorting the string in lexicographic order. For instance, what happens if we want to change the string? Then our sorting might not be correct, depending on how we change the string.

Thus, we are going to try to implement a "fallback" sort criterion for sim-specific percent and common percent. The fallback for sim-specific percent will be the total number of sim-specific strings, and the fallback for common percent will be the total number of common strings. This way if the percentages are equal, the sorting will depend on the total number of strings.

liammulh added a commit that referenced this issue Dec 22, 2022
liammulh added a commit that referenced this issue Dec 23, 2022
liammulh added a commit that referenced this issue Dec 23, 2022
@oliver-phet
Copy link
Contributor

@jbphet and I discussed this yesterday, and we think there might be some issues with sorting the string in lexicographic order. For instance, what happens if we want to change the string? Then our sorting might not be correct, depending on how we change the string.

Thus, we are going to try to implement a "fallback" sort criterion for sim-specific percent and common percent. The fallback for sim-specific percent will be the total number of sim-specific strings, and the fallback for common percent will be the total number of common strings. This way if the percentages are equal, the sorting will depend on the total number of strings.

That sounds fine (and probably more robust).

@jbphet
Copy link
Contributor

jbphet commented Jan 2, 2023

I'm unassigning myself, since @liammulh is planning to implement this (or has already done so).

@jbphet jbphet removed their assignment Jan 2, 2023
liammulh added a commit that referenced this issue Jan 4, 2023
@liammulh
Copy link
Member

liammulh commented Jan 4, 2023

So, for people reading this in the future, the way it should work now is that items with the same percent should have a secondary sort criterion, which is the total number of strings. So if you had something like:

50% (10 of 20)
50% (20 of 40)

The secondary sort criterion should be the 20 and the 40.

Should be resolved as of 006ffca. Marking as ready for review.

@Nancy-Salpepi
Copy link
Author

Nancy-Salpepi commented Jan 20, 2023

In Round 2, there is now only one string column--Translated strings--but the sort button is working correctly:

When pressing the Sort button next to Translated Strings, the sims will sort:
-first by percentage (largest to smallest)
-then within the same percentage, by the total number of strings (largest to smallest)

When pressing that Sort button a second time, the sims will sort:
-first by percentage (smallest to largest)
-then within the same percentage, by the total number of strings (smallest to largest)

Closing.

liammulh added a commit that referenced this issue Feb 8, 2023
liammulh added a commit that referenced this issue Feb 8, 2023
liammulh added a commit that referenced this issue Feb 8, 2023
liammulh added a commit that referenced this issue Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants