Skip to content

Fix43788 removed aria haspopup on the control element#11053

Open
satyammangroliya wants to merge 9 commits into
ILIAS-eLearning:release_10from
satyammangroliya:fix43788-removed-aria-haspopup-on-the-control-element
Open

Fix43788 removed aria haspopup on the control element#11053
satyammangroliya wants to merge 9 commits into
ILIAS-eLearning:release_10from
satyammangroliya:fix43788-removed-aria-haspopup-on-the-control-element

Conversation

@satyammangroliya
Copy link
Copy Markdown
Contributor

aria-haspopup on the control element “Sort test results” is unfavorable
it is noted for the area of the button for sorting test results that the aria-haspopup attribute on the control element is unfavorable, since according to the specification it may only be used for certain types (or roles) of inserted elements (menu, listbox, tree, grid or dialog). It should therefore be removed, as users may otherwise expect a different type of operation.

https://mantis.ilias.de/view.php?id=43788

Copy link
Copy Markdown

@Annett7811 Annett7811 left a comment

Choose a reason for hiding this comment

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

Hello!

From an accessibility perspective, this looks good. Thank you very much for your work.

Best regards,

Annett

Copy link
Copy Markdown
Contributor

@oliversamoila oliversamoila left a comment

Choose a reason for hiding this comment

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

Hello together,
from an UX and UI perspective, the result looks good. Thank you very much for your contribution, @satyammangroliya.
I'm forwarding this to @thibsy for code review. He may contact you with change requests or incorporate the changes directly (That includes cherry picking too.)

Kind regards,
@oliversamoila (as UI coordinator)

@oliversamoila oliversamoila assigned thibsy and unassigned oliversamoila Mar 2, 2026
Copy link
Copy Markdown
Contributor

@thibsy thibsy left a comment

Choose a reason for hiding this comment

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

Hi @satyammangroliya,

Thx a lot for your contribution to the UI framework!

I believe the actual HTML changes are OK – good catch that you adjusted the old and new components here. However, I think the unit tests probably need to be adjusted to these changes as well, otherwise our pipelines fail. For some reason I cannot re-run the pipelines and also not see the logs (due to age of this PR). Could you perform these locally and check?

Kind regards,
@thibsy (as UI coordinator)

@oliversamoila
Copy link
Copy Markdown
Contributor

Hello @satyammangroliya,
please could you provide a brief response to Thibeau’s question.

Many thanks and best regards,
@oliversamoila

@satyammangroliya
Copy link
Copy Markdown
Contributor Author

Dear @thibsy ,
yes i will perform these locally and check .

@satyammangroliya
Copy link
Copy Markdown
Contributor Author

Hello @thibsy ,

Thank you for the hint.

I checked the affected tests locally and found several failing expectations related to the removed aria-haspopup attribute. I updated the corresponding tests and verified the changes locally.

The pipeline is now passing.

Kind regards,
Satyam

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accessibility Pull requests that propose A11Y changes. bugfix kitchen sink

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants