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: Warning about missing label for other answer #1731

Merged
merged 1 commit into from Oct 6, 2023

Conversation

Chartman123
Copy link
Collaborator

NcInputField for other answer throws a warning about missing label prop. As label also works as placeholder, this simply changes the prop for the placeholder text from placeholder to label.

Signed-off-by: Christian Hartmann <chris-hartmann@gmx.de>
@Chartman123 Chartman123 added enhancement New feature or request 3. to review Waiting for reviews labels Sep 29, 2023
@Chartman123 Chartman123 added this to the 3.4 milestone Sep 29, 2023
@Chartman123 Chartman123 self-assigned this Sep 29, 2023
@@ -72,7 +72,7 @@
@keydown.enter.exact.prevent="onKeydownEnter">
{{ t('forms', 'Other:') }}
</NcCheckboxRadioSwitch>
<NcInputField :placeholder="placeholderOtherAnswer"
<NcInputField :label="placeholderOtherAnswer"
Copy link
Collaborator

Choose a reason for hiding this comment

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

No, this makes no sense for accessibility, as the label of the input would be "Enter your answer".
It would make more sense to have a separate label like "Custom answer" or "Other answer" and keep "Enter your answer" as the placeholder.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Or we could change the placeholder text for submitPlaceholder to "Enter your own answer"? This way we wouldn't need another string...

We also have the "other" label for the checkbox in front, so it might already be clear that the following input field belongs to the "other" option. But I'm far from being an a11y expert...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets do this for now. As soon as we use nextcloud/vue 8 we could just use the inner labels and rework this :)

@susnux susnux merged commit 90305f6 into main Oct 6, 2023
22 checks passed
@susnux susnux deleted the fix/warning-missingLabel branch October 6, 2023 16:20
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 enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants