Underline links inside .frm-text-xs help text - #3350
vivi-the-going-merry[bot] wants to merge 3 commits into
Conversation
The GDPR settings knowledgebase link on Global Settings (General) relied on color alone to read as a link against its surrounding grey help text -- contrast falls below 3:1 and there's no underline, an axe link-in-text-block violation. Underline any link inside .frm-text-xs generally, not just this one instance, since the same low-contrast risk applies to any small help text using this utility class. Verified live via formidable-preview-env: before the fix the GDPR link's computed text-decoration-line was "none", after it's "underline".
Self-review turned up two issues with the first pass: - The rule belonged in components/links/_link-styles.scss (next to the existing .frm-link-secondary underline rule), not in a font-size-only utility file. - The broad .frm-text-xs a selector also caught install-addons-step.php's "Connect Account" CTA, an icon+text link with its own distinct treatment that was never meant to look like an inline prose link. Excluded via :not(.frm-link-with-external-icon). Re-verified live via formidable-preview-env: GDPR link still underlines, the "Connect Account"/"Why Upgrade" icon-links stay undecorated.
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Repository: Strategy11/formidable-forms/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| PHP |  |
Sep 20, 2026 2:58a.m. | Review ↗ |
| JavaScript | |
Sep 20, 2026 2:58a.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
![franky-the-going-merry[bot]](https://avatars.githubusercontent.com/in/4233782?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verified live against the PR's own branch in a local WP install (not just the diff): the GDPR settings "here" link now renders underlined, and a synthetic .frm-link-with-external-icon link inside .frm-text-xs correctly stays text-decoration: none (computed styles checked via getComputedStyle, screenshot below) — both branches of the new rule behave as intended.
One non-blocking coverage gap found (inline comment): the selector only reaches an <a> nested inside a .frm-text-xs container, not an <a> that carries the frm-text-xs class itself.
| // needs against the surrounding text -- underline them so the link doesn't | ||
| // rely on color as its only cue. Excludes icon+text CTA links (e.g. "Connect | ||
| // Account"), which already have their own distinct treatment. | ||
| .frm-text-xs a:not(.frm-link-with-external-icon) { |
There was a problem hiding this comment.
Non-blocking: this is a descendant selector, so it only reaches an <a> nested inside a .frm-text-xs element — it doesn't match an <a> that carries the frm-text-xs class on itself. That shape exists in the repo today: js/src/form-templates/elements/applicationTemplatesElement.js's "See all applications" link renders as a({ className: 'frm-text-xs frm-font-semibold', ... }) — same muted small-text styling this PR is meant to fix, but it won't get underlined since it isn't a descendant of a .frm-text-xs container.
| .frm-text-xs a:not(.frm-link-with-external-icon) { | |
| :is(.frm-text-xs, .frm-text-xs a):not(.frm-link-with-external-icon) { | |
| text-decoration: underline; | |
| } |
(only correct if :is() is safe for this build's target browsers/PostCSS setup — otherwise a plain .frm-text-xs a:not(...), a.frm-text-xs:not(...) grouped selector does the same thing.)
There was a problem hiding this comment.
Fixed — added a grouped a.frm-text-xs:not(.frm-link-with-external-icon) selector alongside the descendant one, so an carrying the class directly is covered too. Went with the plain grouped-selector fallback over :is() since this repo's SCSS has no existing :is() usage to confirm build-target safety against.
vivi-the-going-merry
Bot
added
vivi-working
Franky's review on this PR found the selector only reached an <a> nested inside a .frm-text-xs container, missing an <a> that carries the frm-text-xs class directly (e.g. the "See all applications" link in applicationTemplatesElement.js).
|
Method: in-place push |
There was a problem hiding this comment.
Re-review at the new head (1cc7787): the previously-noted non-blocking gap is fixed. .frm-text-xs a:not(...) gained a sibling a.frm-text-xs:not(...) selector for when the anchor itself carries the class.
Live-verified in formidable-preview-env + playwright-cli with a synthetic element covering all four combinations (self-class link, self-class CTA-excluded, nested link, nested CTA-excluded) — all four resolve to the intended text-decoration-line (underline / none / underline / none):
Also re-confirmed the original GDPR settings link still underlines, and grepped the repo for any real <a class="frm-text-xs"> usage — none exists yet, so the new branch is defensive coverage, not yet exercised by real markup; the existing nested-link CTA exclusion (install-addons-step.php's "Connect Account") still resolves correctly. Nothing else outstanding.
Fixes Strategy11/formidable-pro#6663
What was broken
Reported against formidable-pro, but the actual markup is in this repo: the GDPR settings knowledgebase link on Global Settings (General),
.frm-text-xs > a[target="_blank"]inclasses/views/frm-settings/general.php. It relied on color alone to read as a link against its surrounding grey help text — contrast falls below 3:1 with no underline — an axelink-in-text-blockviolation.Fix
Added
.frm-text-xs a:not(.frm-link-with-external-icon) { text-decoration: underline; }inresources/scss/admin/components/links/_link-styles.scss(next to the existing.frm-link-secondaryunderline rule), rather than just the one reported instance — the same low-contrast risk applies to any small help text using.frm-text-xswith an inline link. The:not()exclusion keeps this from also underlininginstall-addons-step.php's "Connect Account"/"Why Upgrade" icon+text CTA links, which have their own distinct treatment and were never meant to read as inline prose links (caught in self-review, not part of the original report).Independent of any admin color-token change elsewhere, per the issue.
Verified
formidable-preview-env+playwright-cli: before the fix, the GDPR link's computedtext-decoration-linewasnone; after,underline. Confirmed the CTA icon-links elsewhere stay undecorated.npx stylelinton both changed SCSS files: clean.npm run build: compiled CSS committed alongside the SCSS source, per this repo's convention (checked prior "Run build" commits).No before/after screenshot attached — verification is via the link's computed style (above) rather than a screenshot; no screenshot hosting mechanism is set up for this agent yet.