Skip to content

Fix TG: Call to Action button links to an empty href when no link is set - #95

Merged
iamprazol merged 4 commits into
developfrom
fix/cta-empty-link
Oct 5, 2026
Merged

iamprazol merged 4 commits into
developfrom
fix/cta-empty-link

Conversation

@rajatgautam755421

@rajatgautam755421 rajatgautam755421 commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Fixes themegrill/accelerate-pro#54 in Free (Pro fix: themegrill/accelerate-pro#130).

Reported by @iamprazol.

Cause: the widget always saves button_url (as "" when left empty), so the isset() fallback to # never applied, and the button rendered href="", which reloads the page.
Fix: empty() instead of isset(), so it falls back to #.

Before After
before after

How to test: add TG: Call To Action with button text and no Button Redirect Link, then inspect the button. Before: href="". After: href="#". With a link set, the link is kept.

Testing done: new @fresh spec widgets/cta-empty-link (REST-built widget, cleaned up; adds tests/e2e/utils/wp.ts admin helpers) fails on develop and passes here; full suite passes.

PHPCS (repo phpcs.xml.dist): 0 new violations; file errors 128 → 127, warnings unchanged (19, pre-existing).

Changelog: Fix - TG: Call to Action button links to an empty href when no link is set.

🤖 Generated with Claude Code

rajatgautam755421 and others added 2 commits October 1, 2026 14:14
…s set

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Cleanup must verify the widget deletion response instead of silently allowing failed cleanup.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
What changed in this PR

Fixes the CTA widget so an unset button link renders href="#" instead of an empty URL.

Changes:

  • Uses empty() for the CTA URL fallback.
  • Adds REST-based E2E regression coverage and WordPress helpers.
  • Registers widget paths in the QA suite.
File Summary
tests/​e2e/​utils/​wp.ts Adds WordPress helpers; cleanup should assert deletion responses to detect failures.
tests/​e2e/​specs/​widgets/​cta-empty-link.spec.ts Adds regression coverage for empty CTA links.
inc/​widgets/​accelerate-call-to-action-widget.php Falls back to # when no CTA URL is set.
.themegrill-qa/​suite.json Maps widget-related paths to the widgets QA area.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/e2e/utils/wp.ts Outdated
Comment on lines +62 to +64
await page.request.post(`/?rest_route=/wp/v2/widgets/${id}&force=true`, {
headers: { "X-WP-Nonce": await restNonce(page), "X-HTTP-Method-Override": "DELETE" },
});

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Valid, fixed in 29d3421. removeWidget() now asserts res.ok() and reports the HTTP status, like deletePage(). Checked: deleting a non-existent widget now fails with "removing test widget … failed: HTTP 404". In addLegacyWidget() the cleanup after a failed placement swallows its own error, so the placement error (the real cause) is the one reported.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@deepench deepench left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 👍

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@iamprazol iamprazol left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM !! 👍

@iamprazol
iamprazol merged commit 170c5d0 into develop Oct 5, 2026
1 check failed
@iamprazol
iamprazol deleted the fix/cta-empty-link branch October 5, 2026 04:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants