-
Notifications
You must be signed in to change notification settings - Fork 852
Feature: Make the on-page popup resizable #904
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRemoved window-size dependent width from the floating toolbar, scoped and converted content styles to flex-based/resizable behavior under a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Summary of ChangesHello @13pathak, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the user experience by making the on-page popup window fully resizable. Previously, the fixed size and lack of resizing options could hinder interaction. The changes introduce dynamic sizing capabilities, coupled with crucial flexbox adjustments to maintain the correct positioning of UI elements, particularly the input box, during resizing. Additionally, the native Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a great enhancement by making the on-page popup resizable. The implementation is solid, correctly using CSS for resizing and flexbox for layout adjustments. The related JavaScript changes to remove the old dynamic sizing logic are also well-executed. I've only found a couple of minor issues related to leftover comments in the SCSS code. Overall, this is a valuable contribution that improves user experience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/content-script/styles.scss (1)
365-368: Consider responsive textarea height.Disabling the textarea's native resize (
resize: none !important) makes sense since the entire window is now resizable. However, the fixed 70px height might not scale well if users resize the window significantly larger or smaller. Consider making the textarea height responsive to the window size using percentage or viewport units.Example approach for responsive height:
- height: 70px; /* <-- SET A FIXED STARTING HEIGHT */ - min-height: 70px; /* <-- SET A MINIMUM HEIGHT */ + height: 20%; /* Scale with window height */ + min-height: 60px; /* Minimum for usability */ + max-height: 150px; /* Prevent it from becoming too large */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
src/components/FloatingToolbar/index.jsx(1 hunks)src/content-script/styles.scss(4 hunks)src/popup/styles.scss(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/popup/**
📄 CodeRabbit inference engine (AGENTS.md)
Extension popup UI should be implemented under
src/popup/
Files:
src/popup/styles.scss
**/*.{js,jsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier to format all JS, JSX, and CSS files (run
npm run prettybefore linting)
Files:
src/components/FloatingToolbar/index.jsx
**/*.{js,jsx,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Follow ESLint React/JSX standards defined in
.eslintrc.json(runnpm run lint)
Files:
src/components/FloatingToolbar/index.jsx
src/components/**
📄 CodeRabbit inference engine (AGENTS.md)
Add new reusable UI components under
src/components/
Files:
src/components/FloatingToolbar/index.jsx
🧠 Learnings (3)
📚 Learning: 2025-09-12T18:14:47.716Z
Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T18:14:47.716Z
Learning: Applies to src/content-script/selection-tools/** : Selection tools: keep UI and logic separate; reuse helpers from `src/utils/`
Applied to files:
src/components/FloatingToolbar/index.jsx
📚 Learning: 2025-09-12T18:14:47.716Z
Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T18:14:47.716Z
Learning: Applies to src/content-script/index.jsx : Main content script entry is `src/content-script/index.jsx`; keep primary injection logic here
Applied to files:
src/components/FloatingToolbar/index.jsxsrc/content-script/styles.scss
📚 Learning: 2025-09-12T18:14:47.716Z
Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T18:14:47.716Z
Learning: Applies to src/content-script/site-adapters/** : Site adapters: keep selectors minimal, use feature detection, and verify on Chromium/Firefox
Applied to files:
src/content-script/styles.scss
🔇 Additional comments (5)
src/popup/styles.scss (1)
42-42: Verify that this width reduction is intentional.This change reduces the extension popup width from 460px to 430px. Since the PR objective is to make the on-page selection window resizable (not the extension popup), please confirm this width adjustment is intentional and explain its purpose.
src/components/FloatingToolbar/index.jsx (1)
93-93: LGTM! Transition to CSS-based sizing is clean.The removal of dynamic width calculation in favor of CSS-controlled sizing (defined in
src/content-script/styles.scss) simplifies the component and enables the resizable window feature. The.chatgptbox-selection-windownow has fixed dimensions (500×450px) withresize: bothapplied via CSS.src/content-script/styles.scss (3)
77-78: LGTM! Essential flex properties for resizable layout.Adding
display: flexandmin-height: 0to the container is crucial for the resizable window feature. Themin-height: 0specifically allows flex children to shrink below their content size when the window is resized smaller, preventing layout overflow.
81-81: LGTM! Flex growth enables proper content area sizing.Setting
flex: 1on.gpt-innerallows it to grow and fill available space within the resizable container, ensuring the content area expands appropriately.
105-106: LGTM! Correct flex setup for scrollable content.The combination of
flex: 1andmin-height: 0on.markdown-bodyenables proper vertical scrolling within the resizable window. Theoverflow-y: auto(line 103) creates a scrollbar when content exceeds the available space.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/content-script/styles.scss (4)
64-82: Consider adding a comment to clarify the distinction between scoped and unscoped variants.The scoped
.resizable-popup .chatgptbox-container(lines 64-74) and unscoped.chatgptbox-container(lines 76-82) have overlapping properties with intentional differences (e.g.,margin-bottom: 0vsmargin-bottom: 20px, presence ofdisplay: flex). A brief comment explaining when each variant applies would improve maintainability.
1397-1401: Investigate the need for!importanton theresizeproperty.The
resize: none !importantdeclaration suggests there may be competing styles elsewhere in the cascade. If possible, refactor the CSS specificity to avoid!important, which makes styles harder to override and maintain.
1403-1416: Use theme variables for button colors to maintain consistency across themes.The
.submit-buttonuses a hardcodedbackground-color: #30a14einstead of leveraging the existing CSS custom properties (e.g.,var(--theme-color)). This breaks theme consistency and may create contrast issues in different theme modes.Consider replacing the hardcoded color with a theme variable or adding a new custom property:
.submit-button { position: absolute; right: 1.1em; bottom: 0.4em; padding: 1px 6px; cursor: pointer; - background-color: #30a14e; - color: white; + background-color: var(--submit-button-bg, #30a14e); + color: var(--submit-button-fg, white); border: 1px solid; border-radius: 6px; border-color: rgba(31, 35, 40, 0.15); font-size: 1em; box-shadow: 0 1px 0 rgba(31, 35, 40, 0.1); }
1418-1428: Add keyboard focus indicators for drag handles.The
.draggableand.dragbarelements lack:focusand:focus-visiblestyles. If these elements are interactive (tab-focusable), they need visible focus indicators to meet accessibility guidelines (WCAG 2.1 SC 2.4.7).Add focus styles:
.draggable { cursor: move; + + &:focus-visible { + outline: 2px solid var(--color-accent-fg); + outline-offset: 2px; + } } .dragbar { cursor: move; width: 42%; height: 12px; border-radius: 10px; background-color: var(--dragbar-color); + + &:focus-visible { + outline: 2px solid var(--color-accent-fg); + outline-offset: 2px; + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/FloatingToolbar/index.jsx(1 hunks)src/content-script/styles.scss(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier to format all JS, JSX, and CSS files (run
npm run prettybefore linting)
Files:
src/components/FloatingToolbar/index.jsx
**/*.{js,jsx,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Follow ESLint React/JSX standards defined in
.eslintrc.json(runnpm run lint)
Files:
src/components/FloatingToolbar/index.jsx
src/components/**
📄 CodeRabbit inference engine (AGENTS.md)
Add new reusable UI components under
src/components/
Files:
src/components/FloatingToolbar/index.jsx
🧠 Learnings (3)
📚 Learning: 2025-09-12T18:14:47.716Z
Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T18:14:47.716Z
Learning: Applies to src/content-script/selection-tools/** : Selection tools: keep UI and logic separate; reuse helpers from `src/utils/`
Applied to files:
src/components/FloatingToolbar/index.jsxsrc/content-script/styles.scss
📚 Learning: 2025-09-12T18:14:47.716Z
Learnt from: CR
Repo: ChatGPTBox-dev/chatGPTBox PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T18:14:47.716Z
Learning: Applies to src/content-script/index.jsx : Main content script entry is `src/content-script/index.jsx`; keep primary injection logic here
Applied to files:
src/components/FloatingToolbar/index.jsx
📚 Learning: 2025-09-06T17:46:04.304Z
Learnt from: PeterDaveHello
Repo: ChatGPTBox-dev/chatGPTBox PR: 886
File: build.mjs:167-175
Timestamp: 2025-09-06T17:46:04.304Z
Learning: For the ChatGPTBox browser extension project, license preservation concerns about third-party dependencies may not be applicable according to the maintainer PeterDaveHello, who indicated this is the "wrong scope" for the project.
Applied to files:
src/content-script/styles.scss
🔇 Additional comments (2)
src/content-script/styles.scss (1)
1469-1476: Verify minimum width behavior on small mobile devices.The
min-width: 350pxconstraint could cause horizontal overflow on devices with viewports narrower than ~389px (350px + margins/padding). Whilemax-width: 90vwhandles the maximum size, it doesn't prevent the minimum from exceeding the viewport on very small screens.Consider testing on small mobile viewports or adjusting the minimum width:
width: 500px; height: 450px; resize: both; overflow: hidden; - min-width: 350px; + min-width: min(350px, 90vw); min-height: 300px; max-width: 90vw; max-height: 90vh;src/components/FloatingToolbar/index.jsx (1)
93-93: LGTM: Clean separation of sizing concerns.Moving from dynamic width calculation to CSS-based sizing via the
resizable-popupclass improves separation of concerns and aligns well with the CSS changes insrc/content-script/styles.scss.
User description
Hello!
I forked this project to make a small improvement. The on-page popup window was a bit too wide and wasn't resizable, which made it hard to use.
What this PR does:
Makes the on-page popup window fully resizable by the user.
Fixes the flexbox layout so that the input box stays at the bottom when resizing, instead of getting hidden.
Removes the default
textarearesize handle from the input box, as the whole window is now resizable.I've attached a GIF of the new behavior. I hope this is a useful contribution!
PR Type
Enhancement
Description
Make on-page popup window fully resizable with drag handles
Fix flexbox layout to keep input box visible when resizing
Remove textarea resize handle, disable default resize behavior
Set fixed starting height and minimum constraints for popup
Remove dynamic window sizing logic from FloatingToolbar component
Diagram Walkthrough
File Walkthrough
styles.scss
Add resizable popup with flexbox layout fixessrc/content-script/styles.scss
display: flexandmin-height: 0to.chatgptbox-containerforproper flex behavior
.gpt-innerfromheight: 100%toflex: 1to allow flexiblesizing
flex: 1andmin-height: 0to.markdown-bodyfor proper flexshrinking
.interact-inputfromheight: 100%to fixedheight: 70pxwithmin-height: 70pxresize: none !importantto disable textarea's default resizehandle
resize: both,width: 500px,height: 450px, and size constraints to.chatgptbox-selection-windowdisplay: flexandflex-direction: columnto.chatgptbox-selection-windowindex.jsx
Remove dynamic window sizing logicsrc/components/FloatingToolbar/index.jsx
useClampWindowSizehookwindowSizestate calculation that dynamically sized the windowwindowSize[0] * 0.4styles.scss
Adjust popup container widthsrc/popup/styles.scss
.container-popup-modewidth from 460px to 430pxSummary by CodeRabbit