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

Add aria label to the ActionInput component #2472

Merged

Conversation

JuliaKirschenheuter
Copy link
Contributor

resolve nextcloud/calendar#3932

ActionInput has been extended with attribute "aria-label"

Signed-off-by: julia.kirschenheuter julia.kirschenheuter@nextcloud.com

ActionInput has been extended with attribute "aria-label"

Signed-off-by: julia.kirschenheuter <julia.kirschenheuter@nextcloud.com>
@JuliaKirschenheuter JuliaKirschenheuter added 3. to review Waiting for reviews accessibility Making sure we design for the widest range of people possible, including those who have disabilities labels Feb 4, 2022
Copy link
Contributor

@tcitworld tcitworld left a comment

Choose a reason for hiding this comment

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

This shouldn't be required. v-bind="$attrs" should foward every other prop on ActionInput, as stated on https://nextcloud-vue-components.netlify.app/#/Components/Actions?id=actioninput

@JuliaKirschenheuter
Copy link
Contributor Author

JuliaKirschenheuter commented Feb 4, 2022

@tcitworld thanks for you mesage!
i have checked it before PR. ActionInput component don't have "inheritAttrs: false" - that means, that each prop will be added to the root element of the component (and it is <li> )
like this
20220204_115944

@tcitworld
Copy link
Contributor

Indeed, so shouldn't we add inheritAttrs: false instead of this prop to match the documented behavior?

@JuliaKirschenheuter
Copy link
Contributor Author

@tcitworld it is a good idea, but this component includes also the behavior of DatetimePicker and Multiselect, and if i reject the inheritance - it could break the behavior of this components. This library will be used in different projects, and i can't be sure that it will still work well

@tcitworld
Copy link
Contributor

In that case we need to update the docs

All undocumented attributes will be bound to the input, the datepicker or the multiselect component, e.g. maxlength, not-before. For the multiselect component, all events will be passed through. Please see the multiselect component's documentation for more details and examples.

/**
* aria-label attribute of the input field
*/
ariaLabel: String,
Copy link
Contributor

Choose a reason for hiding this comment

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

Prop 'ariaLabel' requires default value to be set

the linter wants a default value ;-)

@ChristophWurst ChristophWurst changed the title Enhancement for ActionInput Add aria label to the ActionInput component Feb 8, 2022
attribute "aria-label" has empty default value

Signed-off-by: julia.kirschenheuter <julia.kirschenheuter@nextcloud.com>
Copy link
Contributor

@GretaD GretaD left a comment

Choose a reason for hiding this comment

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

the comments are delivered

@JuliaKirschenheuter JuliaKirschenheuter merged commit 245073d into master Feb 8, 2022
@JuliaKirschenheuter JuliaKirschenheuter deleted the enhancement/3932-action-input-with-aria-label branch February 8, 2022 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews accessibility Making sure we design for the widest range of people possible, including those who have disabilities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Title input field misses a label
5 participants