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

fixed ID 470077. Purpose of the link is not clear in context #2062

Merged
merged 3 commits into from
Feb 1, 2023

Conversation

CrisGuzmanS
Copy link
Contributor

@CrisGuzmanS CrisGuzmanS commented Jan 30, 2023

Hi @tdonohue, I did the activity with ID 470077 found in issue #1190. I will be attentive to any comment 👍 .

References

Description

I modified the text "withdraw..." and "Move..." according to Tim's specifications. Also, I added the aria-label attribute to support people with disabilities. I modified the translation of some countries to adapt to this change.

Instructions for Reviewers

I modified the .json5 files to accommodate the requested change. Also I added an aria-label tag in the buttons requested by Tim so that the aria-label attribute contains the text according to the language.

List of changes in this PR:

  • Changed texts in .json5 files. (item.edit.tabs.status.buttons.withdraw.button and item.edit.tabs.status.buttons.move.button)
  • aria-label added added for 2 buttons in: item-operation.component.html
  1. Login as adminsitrator
  2. Edit any item.
  3. You will see the button texts changed and aria-lables according to the language:
    image
    image
    image

Checklist

  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  • My PR passes ESLint validation using yarn lint
  • My PR doesn't introduce circular dependencies (verified via yarn check-circ-deps)
  • My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
  • If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
  • If my PR fixes an issue ticket, I've linked them together.

@tdonohue tdonohue added bug accessibility component: Item (Archived) Item display or editing labels Jan 30, 2023
@tdonohue tdonohue added this to the 7.5 milestone Jan 30, 2023
@tdonohue tdonohue added the 1 APPROVAL pull request only requires a single approval to merge label Jan 30, 2023
@tdonohue
Copy link
Member

Thanks @CrisGuzmanS ! I've pulled this over to our 7.5 board... will try to find a reviewer to look at this in the next week.

Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

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

👍 Thanks @CrisGuzmanS ! This looks good to me & I tested it and it's working.

One small note. For the future, you don't need to update every assets/i18n/*.json5 file. We only require that you update en.json5 (the default English translation). You are always welcome to update more of them if you wish... so what you did in this PR is OK. But, you don't have to update all of them unless you want to do so.

Thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 APPROVAL pull request only requires a single approval to merge accessibility bug component: Item (Archived) Item display or editing
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants