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

Give feedback on Rosetta 2 #328

Closed
11 of 13 tasks
liammulh opened this issue Dec 15, 2022 · 14 comments
Closed
11 of 13 tasks

Give feedback on Rosetta 2 #328

liammulh opened this issue Dec 15, 2022 · 14 comments
Assignees

Comments

@liammulh
Copy link
Member

liammulh commented Dec 15, 2022

Hey, @oliver-phet, the newest version of Rosetta 2 is live on Ox-Dev. Can you please take a look at it and provide more feedback in this issue? I've included responses to your last round of feedback below. Thanks!

Also, please look over the wireframes and the design doc to see if there's anything I'm overlooking.


  • Top Navigation
    • Remove ‘Translate’ - this is Rosetta 1.0, correct? Should be removed
    • Rename ‘Translation Report’ to ‘Translate’
    • In my design, I had a link to ‘User Guide’ (no ‘Help’ in navigation). Is there a reason we aren’t doing this on the ‘Translate Utility’ page?
      • I changed the design so that it's called 'User Guide', and it's just a link to an external doc.
  • Translation Utility
    • Could we remove the ‘Get Translation Report’ button - just update the page when a locale is selected?
      • Unfortunately, no, this would require a significant amount of work.
    • Rename the header ‘Translation Report’ is really unintuitive. I’d just use “PhET Translation Utility (HTML5)” like my design used.
    • Don’t say “The translation report is finished!”
    • When loading, maybe just have the spinner? Maybe “Translation data loading [spinner]“?
    • How do users navigate back to locale selection screen? (My design had drop down persist)
      • I added link in the navbar.
    • Handle case when no sims untranslated.
    • Message saying all sims translated, check percentages.
    • No sort buttons if one sim.
  • Translation “Form”
@oliver-phet
Copy link
Contributor

Adding link to 2.0: https://ox-dev.colorado.edu/translate

@oliver-phet
Copy link
Contributor

oliver-phet commented Dec 15, 2022

From @liammulh

You can test, but the form needs to be dirty (i.e. you put in a new value) before the button will be enabled. Also, you have to enable pop-ups for Ox-Dev.
You can save, it should work.
You can hit publish, but nothing will happen because I've turned off string commits and build requests.
(Form needs to be dirty for any of the buttons to be enabled.)

  • Open 'User Guide' in new tab target="_blank" (done in 56b1b93)
  • Change page title to "PhET Translation Utility (HTML5)" (currently it's "PhET Translation Tool") (done in 9bacd01)
  • 'Save' presents this dialog to the user If you have a translation saved for acid-base-solutions in locale vi, it will be overwritten. I'm having trouble understanding what this means and how a translator might "have a translations saved". Perhaps I'm not thinking of something, but why do we have this? Wouldn't that translator always be working from the saved translation? (answered in Give feedback on Rosetta 2 #328 (comment))
  • 'Save' currently doesn't actually save anything (done in multiple commits, addressed in Give feedback on Rosetta 2 #328 (comment))
  • Can we hide Chains, Bumper, and Example Simulation? (answered in Give feedback on Rosetta 2 #328 (comment))
  • Is it possible to add "Signed in as: useremail" to the left of "Sign Out" (answered in Give feedback on Rosetta 2 #328 (comment))
  • match website link style (normal:#6c6c6c, visited:#840633, focus:#e01e5a), there aren't really any links within body text so an underline isn't needed on any text links. (answered in Give feedback on Rosetta 2 #328 (comment))
  • match website button style (#521764 Rectangular buttons with stroke, and inverted on hover.) Disabled could probably use the same color you already have #6c757d. Might be able to just reuse the css the website uses.
    image (answered in Give feedback on Rosetta 2 #328 (comment))

@oliver-phet oliver-phet assigned liammulh and unassigned oliver-phet Dec 15, 2022
liammulh added a commit that referenced this issue Dec 15, 2022
Part of addressing feedback in
#328 (comment).
liammulh added a commit that referenced this issue Dec 15, 2022
Part of addressing feedback in
#328.
@liammulh
Copy link
Member Author

liammulh commented Dec 15, 2022

'Save' presents this dialog to the user If you have a translation saved for acid-base-solutions in locale vi, it will be overwritten. I'm having trouble understanding what this means and how a translator might "have a translations saved". Perhaps I'm not thinking of something, but why do we have this? Wouldn't that translator always be working from the saved translation?

The translator can only have one saved translation per ID/sim/locale combo. Thus, if they save a translation, change the translation, and click the save button again, it will overwrite what they currently have saved.

Does that answer your question, @oliver-phet?

@liammulh
Copy link
Member Author

Can we hide Chains, Bumper, and Example Simulation?

These will be hidden in production. Right now it's set to show them because of the configuration.

@liammulh
Copy link
Member Author

Is it possible to add "Signed in as: useremail" to the left of "Sign Out"

Yes, it's possible. I can add it if you feel it's necessary. I thought it was obvious that the email that's displayed next to the person icon is who you're signed in as.

@liammulh
Copy link
Member Author

liammulh commented Dec 15, 2022

match website link style (normal:#6c6c6c, visited:#840633, focus:#e01e5a), there aren't really any links within body text so an underline isn't needed on any text links.

match website button style (#521764 Rectangular buttons with stroke, and inverted on hover.) Disabled could probably use the same color you already have #6c757d. Might be able to just reuse the css the website uses.
image

I thought we discussed not necessarily being able to match the website's styling? It seems like it wasn't an explicit requirement, and the scope on this rewrite/upgrade is already huge, I don't want to add more requirements.

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

'Save' presents this dialog to the user If you have a translation saved for acid-base-solutions in locale vi, it will be overwritten. I'm having trouble understanding what this means and how a translator might "have a translations saved". Perhaps I'm not thinking of something, but why do we have this? Wouldn't that translator always be working from the saved translation?

The translator can only have one saved translation per ID/sim/locale combo. Thus, if they save a translation, change the translation, and click the save button again, it will overwrite what they currently have saved.

Does that answer your question, @oliver-phet?

That's what I would expect, but isn't that behavior implied by "Save"? I'd pull out my hair if every time I saved a file I was asked if I was sure I wanted to save!

Is it possible to add "Signed in as: useremail" to the left of "Sign Out"

Yes, it's possible. I can add it if you feel it's necessary. I thought it was obvious that the email that's displayed next to the person icon is who you're signed in as.

I don't see an email next to the person icon and I think it's nice to show the email that will receive credit when a translation is submitted. If it's not trivial to add don't worry about it.

match website link style (normal:#6c6c6c, visited:#840633, focus:#e01e5a), there aren't really any links within body text so an underline isn't needed on any text links.

match website button style (#521764 Rectangular buttons with stroke, and inverted on hover.) Disabled could probably use the same color you already have #6c757d. Might be able to just reuse the css the website uses.
image

I thought we discussed not necessarily being able to match the website's styling? It seems like it wasn't an explicit requirement, and the scope on this rewrite/upgrade is already huge, I don't want to add more requirements.

That's totally fine. It can be saved for 2.1. It's certainly best for websites to look consistent and I thought it would be pretty easy to add some styling to links/buttons.

@liammulh
Copy link
Member Author

'Save' currently doesn't actually save anything

@oliver-phet, this should be working now. The issue was a corner case I hadn't considered.

There were two issues: (1) I had the environment config flag set to development instead of production and (2) initially website user data can be undefined (if the promise hasn't been fulfilled), so a request for translation form data will have an undefined user ID, so the filter for the database query will be wrong, so it won't get anything from the database. To solve (2), I simply added websiteUserData to the dependency array for the useEffect that gets translation form data.

@liammulh
Copy link
Member Author

That's what I would expect, but isn't that behavior implied by "Save"? I'd pull out my hair if every time I saved a file I was asked if I was sure I wanted to save!

The confirm dialogue is no longer there.

I don't see an email next to the person icon and I think it's nice to show the email that will receive credit when a translation is submitted. If it's not trivial to add don't worry about it.

I added "Signed in as: " before the email.

That's totally fine. It can be saved for 2.1. It's certainly best for websites to look consistent and I thought it would be pretty easy to add some styling to links/buttons.

We actually don't have any styling in Rosetta as it is. We're just using vanilla Bootstrap, and the idea is that we'll just take it out and replace the styling with PhET website styling, but that's actually a big can of worms that I'd rather not open right now.

@liammulh
Copy link
Member Author

Okay, @oliver-phet, as of that last comment, I think I've addressed all of the feedback. Take another look at Ox-Dev when you get a chance.

@oliver-phet
Copy link
Contributor

Looking good - I think just fix the typo in "Singed in as:"

@oliver-phet
Copy link
Contributor

Also, styling in a future version sounds good.

@liammulh
Copy link
Member Author

fix the typo in "Singed in as:"

Done in the latest change and deployed to Ox-Dev.

styling in a future version sounds good

I agree!

@liammulh
Copy link
Member Author

Closing.

liammulh added a commit that referenced this issue Feb 8, 2023
Part of addressing feedback in
#328 (comment).
liammulh added a commit that referenced this issue Feb 8, 2023
Part of addressing feedback in
#328.
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
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants