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

XWIKI-21521: DefaultSkin form should be accessible #2642

Merged
merged 4 commits into from
Feb 12, 2024

Conversation

Sereza7
Copy link
Contributor

@Sereza7 Sereza7 commented Nov 21, 2023

Jira

https://jira.xwiki.org/browse/XWIKI-21521

PR Changes

  • Wrapped the pretty name in a label which is bound to the form field with its for attribute

Note

View

We can see on the screenshot below that an axe-core analysis of the page does not return any violation anymore.
21521-afterPR

* Wrapped the pretty name in a label which is bound to the form field with its `for` attribute
Copy link
Contributor

@michitux michitux left a comment

Choose a reason for hiding this comment

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

Looks good apart from the added ))) where I'm not sure why it was added.

@@ -149,6 +149,7 @@
(((
{{html}}<button class="btn btn-default btn-xs">$services.icon.renderHTML('cross')</button>{{/html}}
)))
)))
Copy link
Contributor

Choose a reason for hiding this comment

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

I cannot see any code that adds the corresponding (((, was this missing before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, it was a mistake, I don't know what I did here...
Addressed in 18cc897 👍

@@ -80,7 +80,7 @@
#set ($class = $doc.getObject($className).xWikiClass)
#foreach ($prop in $class.properties)
#if ($prop.classType != 'TextArea')
; $services.rendering.escape($prop.prettyName, 'xwiki/2.1')
; {{html}}<label for="${className}_0_$escapetool.xml($prop.name)">$escapetool.xml($prop.prettyName)</label>{{/html}}
Copy link
Member

Choose a reason for hiding this comment

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

It's not necessarily the object at index 0 is it? Chances are low but we could imagine that the retrieved object has another index no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just tried using the $prop.number attribute to fill up this id.
Seems like the number used in the ids is not the same, and so it doesn't match.
image
From what I could see in this form, all properties used 0 (because they were the only ones with their name, no duplicate names? ).
I'll be leaving the PR as is for now.

Copy link
Member

Choose a reason for hiding this comment

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

I just tried using the $prop.number attribute to fill up this id.

Yes it won't work with $prop.number: as the name suggest it's the index of the property. Here the 0 is the index of the xobject itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in afe5ee9 👍

Here is the result with the updated template:
21521-wihtoutHardcodedNumber
As we can see, all attribute values reference to the right nodes.

Sereza7 and others added 2 commits February 12, 2024 14:56
* Replaced the hard-coded reference with a call to the object number.
@surli surli merged commit 09c9a0e into xwiki:master Feb 12, 2024
1 check passed
Sereza7 added a commit to Sereza7/xwiki-platform that referenced this pull request Feb 12, 2024
* Wrapped the pretty name in a label which is bound to the form field with its `for` attribute
* Replaced the hard-coded reference with a call to the object number.
Sereza7 added a commit to Sereza7/xwiki-platform that referenced this pull request Feb 12, 2024
* Wrapped the pretty name in a label which is bound to the form field with its `for` attribute
* Replaced the hard-coded reference with a call to the object number.
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