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

Fix aria-describedby for one-itemref/dataobjref components #153

Merged
merged 2 commits into from Jun 23, 2021

Conversation

alex-ball
Copy link
Contributor

If an itemref or dataobjref field is the only field in a workflow component, the aria-describedby property is set to "c[number]_help_[name]" rather than "c[number]_help", because the $one_field_component value is not being passed on to EPrints::MetaField->get_describedby.

This PR adds support for $one_field_component to these two MetaField classes.

The same issue may affect EPrints::MetaField::Multipart->get_basic_input_elements but it is currently awkward for me to test this.

Fix issue where MetaField::Itemref ignores $one_field_component, meaning an incorrect value for aria-describedby is applied when an Itemref is the only field in a component.
Fix issue where MetaField::Dataobjref ignores $one_field_component, meaning an incorrect value for aria-describedby is applied when a Dataobjref is the only field in a component.
@drn05r
Copy link
Contributor

drn05r commented Jun 23, 2021

Thanks Alex. As you say some fields are more difficult to test than others. As non-multiple itemref/dataobjref are not used in a vanilla EPrints install, the issue with aria-describedby not lining up with the ID for the help div got missed. I did take some time trying to deal with the rather inconsistent values for help div IDs, which caused a lot of problems and led to the need for this $one_field_component parameter.

I will review you patches to confirm they fix the issue you describe on my development/test instance of ePrints before accepting the pull request. I will also look into whether I can test the Multipart field as well.

@drn05r drn05r self-requested a review June 23, 2021 15:44
@drn05r drn05r self-assigned this Jun 23, 2021
@drn05r drn05r added the bug Something isn't working label Jun 23, 2021
@drn05r drn05r added this to the 3.4.4 milestone Jun 23, 2021
Copy link
Contributor

@drn05r drn05r left a comment

Choose a reason for hiding this comment

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

I have tested these and they fix the described issues when the two types of MetaField are single as well as multiple

@drn05r drn05r merged commit 01ad0a3 into eprints:master Jun 23, 2021
@alex-ball alex-ball deleted the patch-2 branch June 24, 2021 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants