-
Notifications
You must be signed in to change notification settings - Fork 48
Fix padding bug in chat and search #301
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
Conversation
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.
Caution
Changes requested ❌
Reviewed everything up to 4761334 in 2 minutes and 12 seconds. Click for details.
- Reviewed
368
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
13
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. gui/src/integrations/perplexity/perplexitygui.tsx:45
- Draft comment:
Use consistent double quotes for imports (changed from './Citations' to "./Citations"). - Reason this comment was not posted:
Comment looked like it was already resolved.
2. gui/src/integrations/perplexity/perplexitygui.tsx:262
- Draft comment:
New adjustPadding callback added; verify that the additional 20px offset works well in all layouts. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to verify that the additional 20px offset works well in all layouts. This is asking the author to double-check something, which violates the rules.
3. gui/src/integrations/perplexity/perplexitygui.tsx:274
- Draft comment:
Refactored padding reflow logic by forcing reflow using offsetHeight – ensure this does not impact performance on lower-end devices. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. gui/src/integrations/perplexity/perplexitygui.tsx:333
- Draft comment:
TimelineItem block was refactored; confirm that key props and event handlers (e.g. onToggle, onRetry) are preserved and work as expected. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. gui/src/integrations/perplexity/perplexitygui.tsx:503
- Draft comment:
Passing adjustPadding as onHeightChange to ContinueInputBox – verify that dynamic height adjustments are smooth. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to verify that the dynamic height adjustments are smooth, which falls under the category of asking the author to ensure behavior is intended or tested. This violates the rules.
6. gui/src/pages/gui.tsx:400
- Draft comment:
New inline style for paddingBottom of '10vh' on the last message – validate that using viewport units here produces consistent spacing on different devices. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. gui/src/integrations/perplexity/perplexitygui.tsx:44
- Draft comment:
Use consistent double quotes for imports (changed from single quotes). - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
8. gui/src/integrations/perplexity/perplexitygui.tsx:262
- Draft comment:
Consider extracting the magic offset (20) into a named constant for clarity and easier maintenance. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50% The number 20 is used in multiple places for padding, suggesting it has semantic meaning. Extracting it to a constant would make the meaning clearer and ensure consistency. However, it's a very simple UI padding value that's unlikely to change or cause bugs. The comment is technically correct but may be overly pedantic. The magic number is used consistently and its purpose (padding) is fairly obvious from context. The benefit of extracting it may not justify the added complexity of another named constant. While the context makes the number's purpose clear, having a named constant would still improve readability and make future padding adjustments easier since it would only need to be changed in one place. The comment makes a valid suggestion that would marginally improve code quality, but the benefit is relatively minor given the simplicity of the magic number's usage.
9. gui/src/integrations/perplexity/perplexitygui.tsx:285
- Draft comment:
Accessing offsetHeight to force reflow may be brittle—add a comment explaining why this step is needed. - Reason this comment was not posted:
Comment was on unchanged code.
10. gui/src/pages/gui.tsx:404
- Draft comment:
Verify that using '50vh' for minHeight and '10vh' for paddingBottom provides a responsive layout across different devices. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to verify the responsiveness of the layout, which falls under asking the author to ensure the behavior is intended or tested. This violates the rules.
11. gui/src/integrations/perplexity/perplexitygui.tsx:333
- Draft comment:
Formatting TopGuiDiv with multi-line props improves readability; ensure similar components follow this style consistently. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
12. gui/src/integrations/perplexity/perplexitygui.tsx:121
- Draft comment:
TYPOGRAPHICAL ERROR: The variable 'OFFSET_HERUISTIC' seems to be misspelled. Consider renaming it to 'OFFSET_HEURISTIC' for clarity. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
13. gui/src/pages/gui.tsx:186
- Draft comment:
Typographical Error: The constant name 'OFFSET_HERUISTIC' appears to be misspelled. It likely should be 'OFFSET_HEURISTIC'. Please fix the spelling to improve code clarity. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_J4pZnwbcgvQmKocg
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
@@ -262,6 +260,12 @@ function PerplexityGUI() { | |||
[state.perplexityHistory], | |||
); | |||
|
|||
const adjustPadding = useCallback((height: number) => { |
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.
This is an exact duplicate of the existing adjustPadding
function. Consider importing and reusing the existing function instead.
- function
adjustPadding
(gui.tsx)
Important
Fix padding issue in
PerplexityGUI
andGUI
by adjustingpaddingBottom
based on input container height and session history length.PerplexityGUI
andGUI
by adjustingpaddingBottom
based on input container height and session history length.perplexitygui.tsx
andgui.tsx
.perplexitygui.tsx
andgui.tsx
.This description was created by
for 4761334. You can customize this summary. It will automatically update as commits are pushed.