Add first-run domain scope onboarding#10
Conversation
Explain why JayQuery checks the root domain by default before running the first popup check. The welcome screen lets users choose root domain or, when different, the exact tab hostname. Also keep the scope switch compact by hiding redundant tab-hostname actions and normalising trailing-dot hostnames before comparing targets. Refs LukeSteward#3
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a first-run welcome screen to the popup UI, persists a welcome-seen flag in settings, normalizes trailing dots in hostname resolution, and refactors mode-chip rendering to conditionally display the exact host chip. ChangesFirst-run welcome and mode-chip display
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
entrypoints/popup/style.css (1)
652-655: ⚡ Quick winUse a CSS variable instead of the hardcoded color.
Line 654 uses the hardcoded color
#222936for the hover background, which is inconsistent with the rest of the stylesheet that primarily uses CSS variables likevar(--surface),var(--surface2), etc. Consider using an existing variable or defining a new one if this shade is needed.♻️ Suggested fix
.welcome__btn:hover { border-color: rgba(91, 140, 255, 0.45); - background: `#222936`; + background: var(--surface); }Alternatively, if a slightly lighter shade than
--surfaceis required, consider defining it as a CSS variable for maintainability.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@entrypoints/popup/style.css` around lines 652 - 655, Replace the hardcoded hover background color on .welcome__btn:hover (currently `#222936`) with a CSS variable; either use an existing variable such as var(--surface) or add a new variable (e.g., --surface-hover) to the root and reference that in .welcome__btn:hover to keep styling consistent and maintainable.lib/checkDomain.ts (2)
102-102: 💤 Low valueOptional: The
gflag in the regex is redundant.Since the
$anchor ensures the pattern only matches at the end of the string, the globalgflag has no effect. The regex can be simplified to/\.+$/for clarity.♻️ Simplified regex
- const tab = tabHostname.trim().toLowerCase().replace(/\.+$/g, ''); + const tab = tabHostname.trim().toLowerCase().replace(/\.+$/, '');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/checkDomain.ts` at line 102, The replacement regex uses an unnecessary global flag; update the call that trims trailing dots from tabHostname (the line assigning const tab) to use replace(/\.+$/, '') instead of replace(/\.+$/g, '') so the pattern remains anchored to the end without the redundant 'g' flag; keep the surrounding trim().toLowerCase() chain intact and ensure the variable name 'tab' and its assignment (const tab = ...) are unchanged except for the regex.
102-102: Remove redundant trailing-dot preprocessing; tldts already handles this internally.The
.replace(/\.+$/g, '')is unnecessary. tldts'sgetDomainfunction is designed to handle trailing dots automatically—it returns the expected domain regardless of whether the input has a trailing dot. The preprocessing can be safely removed:const tab = tabHostname.trim().toLowerCase(); const orgDomain = getDomain(tab, { detectIp: false }) ?? tab;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/checkDomain.ts` at line 102, The trailing-dot stripping on tabHostname is redundant; remove the .replace(/\.+$/g, '') call and normalize with trim().toLowerCase(), then pass the resulting tab into getDomain (use getDomain(tab, { detectIp: false }) ?? tab) so tldts handles trailing dots and you still fall back to the original host when getDomain returns null; update the code that sets the tab variable and any subsequent use of orgDomain to use this cleaned tab value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@entrypoints/popup/main.ts`:
- Around line 720-725: The code in dismissWelcomeAndRun currently awaits
saveSettings(settings) so a failure aborts the function and prevents
runCheck(mode) from running; wrap the persistence call in a try/catch (or
otherwise avoid awaiting it) so failures don't block progress: inside
dismissWelcomeAndRun, set settings.firstRunWelcomeSeen, then try { await
saveSettings(settings); } catch (err) { /* log or ignore */ } and afterwards set
currentView = 'main' and await runCheck(mode) so the DNS check always runs even
if saveSettings throws. Ensure you reference the dismissWelcomeAndRun function,
the saveSettings(settings) call, and the runCheck(mode) invocation when making
the change.
---
Nitpick comments:
In `@entrypoints/popup/style.css`:
- Around line 652-655: Replace the hardcoded hover background color on
.welcome__btn:hover (currently `#222936`) with a CSS variable; either use an
existing variable such as var(--surface) or add a new variable (e.g.,
--surface-hover) to the root and reference that in .welcome__btn:hover to keep
styling consistent and maintainable.
In `@lib/checkDomain.ts`:
- Line 102: The replacement regex uses an unnecessary global flag; update the
call that trims trailing dots from tabHostname (the line assigning const tab) to
use replace(/\.+$/, '') instead of replace(/\.+$/g, '') so the pattern remains
anchored to the end without the redundant 'g' flag; keep the surrounding
trim().toLowerCase() chain intact and ensure the variable name 'tab' and its
assignment (const tab = ...) are unchanged except for the regex.
- Line 102: The trailing-dot stripping on tabHostname is redundant; remove the
.replace(/\.+$/g, '') call and normalize with trim().toLowerCase(), then pass
the resulting tab into getDomain (use getDomain(tab, { detectIp: false }) ??
tab) so tldts handles trailing dots and you still fall back to the original host
when getDomain returns null; update the code that sets the tab variable and any
subsequent use of orgDomain to use this cleaned tab value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1b9cc542-a672-4044-b574-38044f1fa051
📒 Files selected for processing (5)
entrypoints/popup/main.tsentrypoints/popup/style.csslib/checkDomain.test.tslib/checkDomain.tslib/settings.ts
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Keep first-run persistence failures from blocking the DNS check, use an existing surface variable for welcome button hover state, and remove the redundant global flag from trailing-dot normalization. Refs LukeSteward#3
|
Absolute Hero, thanks for this, i'll trigger a version change after this, so it will publish these changes! |
2 participants
Summary
Add a one-time first-run welcome screen explaining why JayQuery checks the root domain by default.
Changes
Refs #3
Summary by CodeRabbit
New Features
Improvements
Tests