Skip to content

Missing ARIA label NavBasic, Removing Italian entry from Language Dropdown, Changing translation for Asset #1583

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
merged 5 commits into from
Sep 4, 2020

Conversation

oruburos
Copy link
Collaborator

Addresses some points in discussion #1509
I have verified that this pull request:

  • has no linting errors (npm run lint)
  • is from a uniquely-named feature branch and has been rebased on top of the latest master. (If I was asked to make more changes, I have made sure to rebase onto master then too)
  • is descriptively named

This PR addresses :

  • Missing ARIA label in NavBasic
  • Removing Italian entry from Language Dropdown (it was there to test fallback funcitonality)
  • Changing translation for Asset (Recurso in Spanish Inclusive Language Spanish Translation  #1509 (comment) )
  • Remove unused imports regarding 18n in some files.

Adding ARIA Label in NavTest
Changing translation from Asset to Recurso for spanish
@oruburos
Copy link
Collaborator Author

@andrewn Can you have a look at this, please? Conflicts have been solved and it's up to date.

@andrewn
Copy link
Member

andrewn commented Aug 27, 2020

@oruburos When I load my user's assets page (http://localhost:8000/andrewn/assets), I see a blank screen.

The Web Inspector's console tab shows the following error:

Uncaught TypeError: this.props.t is not a function
AssetList.jsx:89

Can you take a look please?

@catarak
Copy link
Member

catarak commented Sep 2, 2020

i'm really excited to get this in! do y'all think this is the last PR before i can turn the spanish translation on in production?

@oruburos
Copy link
Collaborator Author

oruburos commented Sep 2, 2020

@andrewn Can you have a look at this, please?

Copy link
Member

@andrewn andrewn left a comment

Choose a reason for hiding this comment

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

Great work!

@oruburos
Copy link
Collaborator Author

oruburos commented Sep 3, 2020

@catarak there is work to do to have a complete translation, specifically the issues mentioned in #1584 and #1587, but I think that in the current state is helpful.

@andrewn andrewn merged commit b7d17a0 into processing:develop Sep 4, 2020
@andrewn
Copy link
Member

andrewn commented Sep 4, 2020

@catarak My feeling is that we can turn this on and address the two other issues soon.

@catarak
Copy link
Member

catarak commented Sep 4, 2020

@catarak My feeling is that we can turn this on and address the two other issues soon.

Makes sense to me! I think it's okay to have the translation mostly done and to start getting feedback from users.

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.

3 participants