-
Notifications
You must be signed in to change notification settings - Fork 696
fix(ticket-detail): Make main view scrollable with accessiblity tools #2718
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
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the semantic HTML structure in the mobile ticket agent view by replacing a generic div element with a more semantic main element to better represent the primary content area of the page.
- Changed container element from
divtomainfor improved semantic HTML
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot is there a better way to do this? |
|
@balamurali27 I've opened a new pull request, #2724, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <article | ||
| ref="scrollableDiv" | ||
| :style="`maskImage: ${maskStyle}`" | ||
| @scroll="updateMaskStyle" | ||
| > | ||
| <slot></slot> | ||
| </div> | ||
| </article> |
Copilot
AI
Nov 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using <article> as the root element is semantically incorrect for a generic scrollable container component. The <article> tag should represent self-contained, independently distributable content. This component is used in multiple contexts (activity lists, filter controls) where <article> semantics don't apply. Consider using a more appropriate semantic tag like <section> if semantic markup is desired, or keep it as <div> since it's a generic utility component.
Tested with https://addons.mozilla.org/en-US/firefox/addon/vimium-ff/