Repository navigation
Alt Text form: add ignore class when fixing style_background_decorative - #1258
Open
monsterlag wants to merge 1 commit into
Open
monsterlag wants to merge 1 commit into
monsterlag wants to merge 1 commit into
Conversation
…form The Equal Access style_background_decorative rule flags any element with an inline CSS background image, even after UDOIT marks it as decorative or adds alt text, so the issue came back on every scan. When either option is chosen for this rule, also add the udoit-ignore-style-background-decorative class so the report skips the element afterwards. Fixes ucfopen#1231
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
AI-generated, not reviewed by a human. This pull request was written by an AI coding agent and then checked by a separate AI review agent, which re-ran the tests described below. No human has reviewed the code. It is submitted by @monsterlag, who runs these agents. If that does not fit how you want to receive contributions, please feel free to close it.
Fixes #1231
What was wrong
The Equal Access
style_background_decorativerule fails any element whose inlinestylehas a CSS background with aurl(...). It does not look atalt,titleorrole="presentation". So when the Alt Text form fixed one of these issues (for example by marking the background as decorative, which addsrole="presentation" alt="" title=""), the next scan reported the same element again, indefinitely.Fix
As suggested in the issue, when the Alt Text form is used on a
style_background_decorativeissue and the user picks "Add text" or "Mark as decorative", the form now also adds theudoit-ignore-style-background-decorativeclass (the issue's existingignoreClass) to the element.Report.jsalready skips unresolved issues whose element carriesudoit-ignore-[rule-id], so the issue no longer reappears after it has been fixed. If no option is selected the class is removed again. Other alt text rules are unchanged.Changes are in
assets/js/Components/Forms/AltTextForm.js, plus a new test,assets/js/__tests__/AltTextForm.test.js, that renders the form and checks the HTML it produces.How tested
assets/js/__tests__/AltTextForm.test.js(4 tests). Before the change, the two tests that expect the ignore class failed (Tests: 2 failed, 2 passed). After it, all 4 pass.jest-environment-jsdomis not a dependency (Jest 30 no longer bundles it), andjest.setup.jsloads MSW, which throwsReferenceError: Response is not definedunder jsdom. So I ran Jest with a temporary config outside the repo: the sametestMatchandmoduleNameMapper,jest-environment-jsdom@30installed separately, and a setup file that loads onlyregenerator-runtimeand@testing-library/jest-dom. Result:AltTextForm.test.jspassed (4/4). The existingApp.test.jsstill fails to load, with the same error before and after this change (SyntaxError: Cannot use import statement outside a modulefrom@wavesurfer/react). That failure is unrelated to this PR.npx eslint assets/js/Components/Forms/AltTextForm.js: the only errors are the twono-useless-escapeerrors that are already ondev, now on a different line number. On the new test file, eslint reportsno-undeffor the Jest globals (describe,it,expect,jest), as it does for the existingApp.test.js, because the eslint config does not declare Jest globals.npx encore production: compiled successfully.I did not test this against a live LMS course or a real Equal Access scan.