Skip to content

Infotip - Clear earlier infotip content field while creating the next one#57

Merged
Verma-Punit merged 4 commits intomainfrom
bugfix/56-clear-format-content-before-switching
Jul 6, 2025
Merged

Infotip - Clear earlier infotip content field while creating the next one#57
Verma-Punit merged 4 commits intomainfrom
bugfix/56-clear-format-content-before-switching

Conversation

@nagpai
Copy link
Contributor

@nagpai nagpai commented Jul 2, 2025

Summary

Fixes the bug within the editor, where text content from an earlier tooltip persisted within the popover, when we moved to the next tooltip.

Fixes another bug where the earlier tooltip remained open when subsequent tooltips are added.

PR adds a useEffect to hide all tooltips before adding a new one.

Related Issue

#57

Steps to test

  • Create a test site using the playground link on the PR
  • Edit any post or page and apply the infotip format
  • Add content and select a different text to add another infotip
  • When you click to add, the popover should not show the earlier infotip content, rather should show the placeholder text
  • When you add new text the older infotip should no longer be visible

Screenshots / Screen Recording / Logs

infotip-clearup-test-1751624920462.mp4

@github-actions
Copy link

github-actions bot commented Jul 2, 2025

Preview via Cloudflare R2 Storage

⚡️WordPress Playground Preview
🚀 Build zip file Download

I will update this comment with the latest preview links as you push more changes to this PR.

Note

The preview sites are created using WordPress Playground. You can add content, edit settings, and test the pull request as you would on a real site, but please note that changes are not saved between sessions.

@nagpai nagpai linked an issue Jul 2, 2025 that may be closed by this pull request
@nagpai nagpai requested a review from Copilot July 3, 2025 12:17
Copy link

Copilot AI left a 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 refactors the infotip inline UI and web component to extract constants and handlers, add clear/reset functionality in each tab, and modularize rendering and update logic.

  • Extracted PLACEMENT_OPTIONS and ICONS constants; replaced inline lambdas with named handlers in Text, Overlay, and Icon tabs
  • Fixed removeAttributes to avoid mutating activeAttributes and introduced default-type assignment
  • Modularized infotip-web-component.js with renderElement, updateIcon, positionArrow, and a switch-based attributeChangedCallback

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/infotip/inline-ui.js Extracted constants, DRY’d out handlers, fixed attribute mutation, added clear/reset handlers
assets/infotip/infotip-web-component.js Broke out template, icon, position and style logic into methods; unified attribute change logic
Comments suppressed due to low confidence (2)

src/infotip/inline-ui.js:164

  • [nitpick] Variable overLaySettingsEnabled has inconsistent casing for “overlay”; consider renaming to overlaySettingsEnabled for clarity and consistency.
	const overLaySettingsEnabled =

src/infotip/inline-ui.js:117

  • [nitpick] New clear/reset handlers have been introduced; ensure you add or update unit tests to cover handleClear, handleReset, and similar reset flows.
	const handleClear = () => {

Comment on lines 346 to 348
if ( iconEnabled && ! activeAttributes[ 'icon-type' ] ) {
updateAttributes( {
'icon-type': 'info',
} );
updateAttributes( { 'icon-type': 'info' } );
}
Copy link

Copilot AI Jul 3, 2025

Choose a reason for hiding this comment

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

Avoid calling updateAttributes during render; side effects in render can trigger infinite loops. Move the default icon-type assignment into a useEffect or initialization step.

Copilot uses AI. Check for mistakes.
@nagpai nagpai marked this pull request as ready for review July 3, 2025 13:52
@nagpai nagpai changed the title WIP - Clear formatting from settings Infotip - Clear earlier infotip content field while creating the next one Jul 4, 2025
@nagpai nagpai requested review from Verma-Punit and ajitbohra and removed request for ajitbohra July 4, 2025 10:31
Copy link
Contributor

@Verma-Punit Verma-Punit left a comment

Choose a reason for hiding this comment

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

@nagpai Looks Good ✨

@Verma-Punit Verma-Punit merged commit fa803fe into main Jul 6, 2025
2 checks passed
@Verma-Punit Verma-Punit deleted the bugfix/56-clear-format-content-before-switching branch July 6, 2025 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Infotip: Format Not Clearing on New Text Selection

3 participants