-
Notifications
You must be signed in to change notification settings - Fork 100
Patch: UI improvement #212
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
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
Reviewer's GuideThis PR refactors the popup UI for better consistency by standardizing label fonts, spacing, and button layouts, fixes the home button hover title, introduces a revamped Notes section, and bumps the extension version to 2.0.0. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @vedansh-5 - I've reviewed your changes - here's some feedback:
- Inline style attributes (e.g. margin-left) break consistency with Tailwindβreplace them with the appropriate utility classes.
- The Notes section uses
tags inside a
- instead of
- elements, so switch to proper list items for correct semantics.
- Using arbitrary spacing classes like gap-[7px] can be confusing; consider sticking to the standard Tailwind spacing scale (e.g. gap-2 or gap-1.5).
Prompt for AI Agents
Please address the comments from this code review: ## Overall Comments - Inline style attributes (e.g. margin-left) break consistency with Tailwindβreplace them with the appropriate utility classes. - The Notes section uses <p> tags inside a <ul> instead of <li> elements, so switch to proper list items for correct semantics. - Using arbitrary spacing classes like gap-[7px] can be confusing; consider sticking to the standard Tailwind spacing scale (e.g. gap-2 or gap-1.5).
Help me be more useful! Please click π or π on each comment and I'll use the feedback to improve your reviews.
|
@hpdang I have made the mentioned changes excluding the Please explain this, I could not see how you want it, if we have all of them in one line, custom date selection wouldn't look so good. |
|
@vedansh-5 I still see that Organization Name and Project Name look bold compare to rest Regarding the "Fetch your contributions" section, Can we please try to put all date related to one line
|
|
@hpdang Thanks for the feedback! Regarding the Organization Name and Project Name: all of them use the same style classes. In your screenshot it appears bolder, but on my end it renders uniformly - so it might be a local rendering issue on your side (browser/font/rendering engine). |
|
@mariobehling any feedback here? |
|
@vedansh-5 maybe we don't need the last 7 days, either previous day or if they want a period, they should select. What do you think? |
|
@vedansh-5 and for the Note part, what do you think about this "Pull requests may appear in your report based on recent activity by others, not just your own. Please review and edit the generated SCRUM to ensure accuracy before sharing." |
We can get rid of last 7 days, its good for making weekly reports though. |
Instead of both the points we can have this one, but this does not provide any description as to why this would happen. |
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
|
@hpdang I have made the changes, please review, I haven't changed the notes yet, how do you want me to keep them? Thanks! |
Signed-off-by: Vedansh Saini <[email protected]>
|
@vedansh-5 I have the following suggestions: Email subject
Fetch contributions
NotesReplace the existing one with "Pull requests are fetched based on the most recent review activity by any contributor, not just your own. Please review and adjust the generated SCRUM report to ensure it accurately reflects your work before sharing." Setting screen
|
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
|
@hpdang Made the suggested changes, please review, thanks. |
hpdang
left a comment
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.
LGTM! Thank you
|
@vedansh-5 one thing, when the Project Name is blank, the Compose Window Subject show "[Scrum] - - 20250728" (can we please remove this when it is bank? |
should this skip the subject generation at all? |
Signed-off-by: Vedansh Saini <[email protected]>
|
@vedansh-5 the subject should still be there with Scrum + Date |
Signed-off-by: Vedansh Saini <[email protected]>
|
@hpdang Made the required change. |




π Fixes
Fixes #211
π Summary of Changes
Consistent labels
Proper spacing
Notes section changes
Show section changes
Hover title fix
πΈ Screenshots / Demo (if UI-related)
β Checklist
π Reviewer Notes
Add any special notes for the reviewer here
Summary by Sourcery
Improve the popup UI by standardizing label styles and spacing, fixing a hover title bug, consolidating the Notes section, and updating the extension version.
New Features:
Bug Fixes:
Enhancements:
Build: