Change label element from div to label for accessibility#68
Conversation
|
Hi! I'm VTEX IO CI/CD Bot and I'll be helping you to publish your app! 🤖 Please select which version do you want to release:
And then you just need to merge your PR when you are ready! There is no need to create a release commit/tag.
|
|
Beep boop 🤖 I noticed you didn't make any changes at the
In order to keep track, I'll create an issue if you decide now is not a good time
|
|
Hi, @RichardByrneCP |
|
Hi @vsseixaso, apologies! Have added the second PR to the description. |
|
Hi @vsseixaso, could you please review this as a priority as it has been 2 weeks since I updated the PR. |
There was a problem hiding this comment.
Pull Request Overview
This PR improves web accessibility for the NumberStepper component by replacing a div element with a proper label element and establishing the correct association between the label and input field.
- Changes div element to label element for the quantity selector title
- Adds htmlFor attribute to associate the label with the quantity input field
- Updates changelog to document the accessibility improvement
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| react/components/BaseProductQuantity.tsx | Replaces div with label element and adds htmlFor attribute for accessibility |
| CHANGELOG.md | Documents the web accessibility improvement |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Hi @RichardByrneCP, Based on the issue raised from your contributions, we’ve created the
You can use this flag whenever you want to implement semantic HTML improvements that might break other clients’ styling, and enable it only for stores that support the change. This allows a gradual migration without causing regressions in production. The public documentation is not ready yet, so I’m attaching the internal usage documentation of the feature to help you get started. |
Accessibility Feature Flag: a11ySemanticHtmlMigration
What it isA feature flag that controls accessibility modifications that introduce breaking changes through HTML structure changes in components. Goal: Allow gradual migration to semantic HTML without breaking existing implementations. Configuration1. Manifest.json{
"dependencies": {
"vtex.render-runtime": "8.x"
}
}2. How to enableAdmin > Apps > My Apps > VTEX Store > Settings > "Enable accessibility semantic HTML migration" How to useimport React from 'react'
import { useRuntime } from 'vtex.render-runtime'
function ProductQuantity() {
const { getSettings } = useRuntime()
const settings = getSettings('vtex.store')
const useSemanticHtml = settings?.advancedSettings?.a11ySemanticHtmlMigration
return (
<>
{useSemanticHtml ? (
<label htmlFor="quantity-input">Quantity:</label>
) : (
<div>Quantity:</div>
)}
<input
id={useSemanticHtml ? "quantity-input" : "qty-123"}
type="number"
/>
</>
)
}Benefits
|
vsseixaso
left a comment
There was a problem hiding this comment.
Hi @RichardByrneCP,
For this PR, the use of the a11ySemanticHtmlMigration flag will be required (more details can be found in the discussions of this PR [1] [2]).
The associated Styleguide PR belongs to another team; their review has already been requested. Once they approve it and the updates here are merged, we can finalize this PR.
|
@vsseixaso Thanks for the details on the new feature flag. Have implemented it into both the PRs and is working on the workspace linked in the description. |
vsseixaso
left a comment
There was a problem hiding this comment.
The implementation is working. The only point that needs correction is the lint errors in the react/components/BaseProductQuantity.tsx file.
| {...labelProps} | ||
| > |
There was a problem hiding this comment.
There's still a lint error here that needs to be fixed.
There was a problem hiding this comment.
@vsseixaso Format for linting issues is a bit unclear. Does this mean to put {...labelProps} on line 84?
There was a problem hiding this comment.
Actually, think it's asking for the closing '>' tag to be put on the previous line. Have added it in a new commit, Please retry linting and let me know.
#### What is the purpose of this pull request? This is the second of two PRs to solve a web accessibility issue for the NumberStepper input and associated label. The first PR is here: vtex-apps/product-quantity#68 This PR adds the id of "vtex-product-quantity-input" to the input element, which is labelled by the label element created in the first PR link above. #### How to test it? Check the HTML in developer console to see the changes reflected. [Workspace](https://accessibilitytesting--colprofr.myvtex.com/elmex-kids-junior-toothpaste-61044604/p) #### Screenshots or example usage: <img width="1726" height="912" alt="Screenshot 2025-08-19 at 09 54 56" src="https://github.com/user-attachments/assets/469cd6b4-046f-489f-906f-6474627f9962" /> #### Screenshots or example usage #### Types of changes - [x] Bug fix (a non-breaking change which fixes an issue) - [ ] New feature (a non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Requires change to documentation, which has been updated accordingly.
|
Your PR has been merged! App is being published. 🚀 After the publishing process has been completed (check #vtex-io-releases) and doing A/B tests with the new version, you can deploy your release by running:
After that your app will be updated on all accounts. For more information on the deployment process check the docs. 📖 |
What problem is this solving?
This is one of two PRs to solve a web accessibility issue for the NumberStepper input and associated label. The second PR is here: vtex/styleguide#1454
This PR makes 2 changes to the label within the product-quantity app:
How to test it?
Check the HTML in developer console to see the changes reflected.
Workspace
Screenshots or example usage: