539 implement tours in extension #550
Conversation
WalkthroughThis pull request enhances a web extension by introducing new UI elements and interactivity features. In Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant FM as createFloatingMenu()
participant GL as generateList()
participant AD as addDragEvent()
U->>FM: Initiate floating menu
FM->>FM: Check if menu exists and create if not
U->>FM: Click mode button ("tour")
FM->>GL: Generate list for tour steps
U->>AD: Drag tour steps to reorder
AD->>FM: Update steps order
sequenceDiagram
participant U as User
participant AB as Action Button
participant BG as setup()
U->>AB: Click extension action button
AB->>BG: Trigger setup in current tab
BG->>BG: Initialise highlight and event listeners (mousemove, click, keydown)
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
backend/.env (1)
9-14:⚠️ Potential issueSecurity: Email credentials exposed in version control.
Email credentials and app passwords should not be committed to the repository.
Move these sensitive values to a secure environment variable management system and use placeholders in the example file:
-[email protected] -EMAIL_PASSWORD=passwor -EMAIL_HOST=smtp.gmail.com -EMAIL_PORT=465 -APP_PASSWORD=ukzwakckupguegiw +[email protected] +EMAIL_PASSWORD=your_email_password +EMAIL_HOST=your_smtp_host +EMAIL_PORT=your_smtp_port +APP_PASSWORD=your_app_password
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
extention/icon.pngis excluded by!**/*.png
📒 Files selected for processing (4)
backend/.env(2 hunks)extention/app.js(1 hunks)extention/background.js(1 hunks)extention/manifest.json(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
extention/app.js
[error] 460-460: This aliasing of this is unnecessary.
Arrow functions inherits this from their enclosing scope.
Safe fix: Use this instead of an alias.
(lint/complexity/noUselessThisAlias)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (22.x)
There was a problem hiding this comment.
- There might be a problem with step naming -It follows the consecutive order no matter what. I think it would be better if we just use the target element names rather than Step counts.
- Also I know you didn't do the copy button below, but can you change its color to 'guidefox purple'?
Steps
Initial:
Add a step:
Move items:
|
Also I sometimes get |
…-onboarding into 539-implement-tours-in-extension
…g into 539-implement-tours-in-extension
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
extention/app.js (2)
332-336:⚠️ Potential issueHeads up! XSS vulnerability in HTML template.
The
cardTemplateuses raw HTML which could be dangerous ifstepcontains malicious content.-const cardTemplate = `<i class="${dragClass}">≡</i><span>{{step}}</span><i class="${trashClass}"><img src="https://cdn-icons-png.flaticon.com/512/860/860829.png" ald="Delete" width="16px" height="16px" /></i>`; +const cardTemplate = document.createElement('div'); +const dragIcon = document.createElement('i'); +dragIcon.className = dragClass; +dragIcon.textContent = '≡'; +const stepSpan = document.createElement('span'); +stepSpan.textContent = `${step}`; +const trashIcon = document.createElement('i'); +trashIcon.className = trashClass; +const img = document.createElement('img'); +img.src = 'https://cdn-icons-png.flaticon.com/512/860/860829.png'; +img.alt = 'Delete'; +img.width = 16; +img.height = 16;
475-493:⚠️ Potential issueYo! Memory leak and unnecessary this aliasing in throttle function.
The throttle function creates new DOM elements on every call and uses unnecessary this aliasing.
function throttle(func, limit = 100) { - createStickyDiv(); - createFloatingMenu(); let inThrottle; let lastResult; - domSelected = false; return function () { const args = arguments; - const context = this; if (!inThrottle) { inThrottle = true; setTimeout(() => (inThrottle = false), limit); - lastResult = func.apply(context, args); + lastResult = func.apply(this, args); } return lastResult; }; }🧰 Tools
🪛 Biome (1.9.4)
[error] 483-483: This aliasing of this is unnecessary.
Arrow functions inherits
thisfrom their enclosing scope.
Safe fix: Use this instead of an alias.(lint/complexity/noUselessThisAlias)
🧹 Nitpick comments (3)
extention/app.js (3)
1-12: Yo! Consider using an enum or object for modes.Instead of using an array for
AVAILABLE_MODES, consider using an enum or object to prevent typos and enable better IDE support.-const AVAILABLE_MODES = ["hint", "tour"]; +const AVAILABLE_MODES = { + HINT: "hint", + TOUR: "tour" +} as const;
301-322: Yo! Consider using ResizeObserver for dynamic updates.The
updateHighlightfunction might miss updates when elements change size dynamically.+const resizeObserver = new ResizeObserver(entries => { + entries.forEach(entry => { + if (entry.target === lastHighlightTarget) { + updateHighlight({ target: lastHighlightTarget }); + } + }); +}); function updateHighlight({ target }) { if ( !(target instanceof HTMLElement) || target === lastHighlightTarget || target.id.startsWith("bw-") ) { return; } lastHighlightTarget = target; + if (lastHighlightTarget) { + resizeObserver.observe(lastHighlightTarget); + } const { top, left, width, height } = target.getBoundingClientRect();
399-420: Yo! Consider adding error boundaries for tour mode.The
grabSelectorfunction should handle errors gracefully in tour mode to prevent tour corruption.if (selectedMode === "tour") { + try { const selector = generateSelector(target); + if (!selector) { + console.warn('Failed to generate selector for element'); + return; + } selectedElements.push({ element: selector, step: selectedElements.length + 1, }); generateList(); + } catch (error) { + console.error('Failed to add tour step:', error); + } return; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
extention/trash.pngis excluded by!**/*.png
📒 Files selected for processing (1)
extention/app.js(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
extention/app.js
[error] 483-483: This aliasing of this is unnecessary.
Arrow functions inherits this from their enclosing scope.
Safe fix: Use this instead of an alias.
(lint/complexity/noUselessThisAlias)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (22.x)
🔇 Additional comments (1)
extention/app.js (1)
272-282:⚠️ Potential issueYo dawg! Duplicate event listeners on button.
The mouseenter, mouseleave, and click event listeners are duplicated for the button.
Remove the duplicate event listeners:
- button.addEventListener("mouseenter", () => { - button.style.backgroundColor = "#6941C6"; - }); - button.addEventListener("mouseleave", () => { - button.style.backgroundColor = "#7F56D9"; - }); - button.addEventListener("click", async () => { - await navigator?.clipboard?.writeText(input.value); - //alert(`You entered: ${input.value}`); - }); - stickyDiv.appendChild(button);Likely an incorrect or invalid review comment.
Describe your changes
Add the floating menu to the page with the possibility of selecting just one element for hint, or multiple elements for tours.
Issue number
#539
Please ensure all items are checked off before requesting a review: