feat: #102 — chore(storybook-screenshots): refresh baseline + investigate duplicate-baseline stories#111
Open
luandro wants to merge 3 commits into
Open
feat: #102 — chore(storybook-screenshots): refresh baseline + investigate duplicate-baseline stories#111luandro wants to merge 3 commits into
luandro wants to merge 3 commits into
Conversation
… + investigate duplicate-baseline stories
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
42
to
132
| /** Default copy button with a URL displayed above */ | ||
| export const Default: Story = { | ||
| args: { | ||
| text: 'https://example.com/invite/abc123', | ||
| }, | ||
| render: (args) => ( | ||
| <div | ||
| style={{ | ||
| display: 'flex', | ||
| flexDirection: 'column', | ||
| gap: 8, | ||
| alignItems: 'center', | ||
| }} | ||
| > | ||
| <code | ||
| style={{ | ||
| fontSize: 13, | ||
| color: '#172033', | ||
| background: '#F4F6FA', | ||
| padding: '4px 8px', | ||
| borderRadius: 6, | ||
| }} | ||
| > | ||
| {args.text} | ||
| </code> | ||
| <CopyButton {...args} /> | ||
| </div> | ||
| ), | ||
| }; | ||
|
|
||
| /** Short text content */ | ||
| /** Short text content displayed above */ | ||
| export const ShortText: Story = { | ||
| args: { | ||
| text: 'abc', | ||
| }, | ||
| render: (args) => ( | ||
| <div | ||
| style={{ | ||
| display: 'flex', | ||
| flexDirection: 'column', | ||
| gap: 8, | ||
| alignItems: 'center', | ||
| }} | ||
| > | ||
| <code | ||
| style={{ | ||
| fontSize: 13, | ||
| color: '#172033', | ||
| background: '#F4F6FA', | ||
| padding: '4px 8px', | ||
| borderRadius: 6, | ||
| }} | ||
| > | ||
| {args.text} | ||
| </code> | ||
| <CopyButton {...args} /> | ||
| </div> | ||
| ), | ||
| }; | ||
|
|
||
| /** Custom label and success label */ | ||
| /** Custom label and success label displayed with URL */ | ||
| export const CustomLabels: Story = { | ||
| args: { | ||
| text: 'https://example.com/invite/abc123', | ||
| label: 'Copy link', | ||
| successLabel: 'Link copied!', | ||
| }, | ||
| render: (args) => ( | ||
| <div | ||
| style={{ | ||
| display: 'flex', | ||
| flexDirection: 'column', | ||
| gap: 8, | ||
| alignItems: 'center', | ||
| }} | ||
| > | ||
| <code | ||
| style={{ | ||
| fontSize: 13, | ||
| color: '#172033', | ||
| background: '#F4F6FA', | ||
| padding: '4px 8px', | ||
| borderRadius: 6, | ||
| }} | ||
| > | ||
| {args.text} | ||
| </code> | ||
| <CopyButton {...args} /> | ||
| </div> | ||
| ), | ||
| }; |
There was a problem hiding this comment.
Suggested change
| /** Default copy button with a URL displayed above */ | |
| export const Default: Story = { | |
| args: { | |
| text: 'https://example.com/invite/abc123', | |
| }, | |
| render: (args) => ( | |
| <div | |
| style={{ | |
| display: 'flex', | |
| flexDirection: 'column', | |
| gap: 8, | |
| alignItems: 'center', | |
| }} | |
| > | |
| <code | |
| style={{ | |
| fontSize: 13, | |
| color: '#172033', | |
| background: '#F4F6FA', | |
| padding: '4px 8px', | |
| borderRadius: 6, | |
| }} | |
| > | |
| {args.text} | |
| </code> | |
| <CopyButton {...args} /> | |
| </div> | |
| ), | |
| }; | |
| /** Short text content */ | |
| /** Short text content displayed above */ | |
| export const ShortText: Story = { | |
| args: { | |
| text: 'abc', | |
| }, | |
| render: (args) => ( | |
| <div | |
| style={{ | |
| display: 'flex', | |
| flexDirection: 'column', | |
| gap: 8, | |
| alignItems: 'center', | |
| }} | |
| > | |
| <code | |
| style={{ | |
| fontSize: 13, | |
| color: '#172033', | |
| background: '#F4F6FA', | |
| padding: '4px 8px', | |
| borderRadius: 6, | |
| }} | |
| > | |
| {args.text} | |
| </code> | |
| <CopyButton {...args} /> | |
| </div> | |
| ), | |
| }; | |
| /** Custom label and success label */ | |
| /** Custom label and success label displayed with URL */ | |
| export const CustomLabels: Story = { | |
| args: { | |
| text: 'https://example.com/invite/abc123', | |
| label: 'Copy link', | |
| successLabel: 'Link copied!', | |
| }, | |
| render: (args) => ( | |
| <div | |
| style={{ | |
| display: 'flex', | |
| flexDirection: 'column', | |
| gap: 8, | |
| alignItems: 'center', | |
| }} | |
| > | |
| <code | |
| style={{ | |
| fontSize: 13, | |
| color: '#172033', | |
| background: '#F4F6FA', | |
| padding: '4px 8px', | |
| borderRadius: 6, | |
| }} | |
| > | |
| {args.text} | |
| </code> | |
| <CopyButton {...args} /> | |
| </div> | |
| ), | |
| }; | |
| function CopyButtonWithLabel(args: React.ComponentPropsWithoutRef<typeof CopyButton>) { | |
| return ( | |
| <div | |
| style={{ | |
| display: 'flex', | |
| flexDirection: 'column', | |
| gap: 8, | |
| alignItems: 'center', | |
| }} | |
| > | |
| <code | |
| style={{ | |
| fontSize: 13, | |
| color: '#172033', | |
| background: '#F4F6FA', | |
| padding: '4px 8px', | |
| borderRadius: 6, | |
| }} | |
| > | |
| {args.text} | |
| </code> | |
| <CopyButton {...args} /> | |
| </div> | |
| ); | |
| } | |
| /** Default copy button with a URL displayed above */ | |
| export const Default: Story = { | |
| args: { | |
| text: 'https://example.com/invite/abc123', | |
| }, | |
| render: (args) => <CopyButtonWithLabel {...args} />, | |
| }; | |
| /** Short text content displayed above */ | |
| export const ShortText: Story = { | |
| args: { | |
| text: 'abc', | |
| }, | |
| render: (args) => <CopyButtonWithLabel {...args} />, | |
| }; | |
| /** Custom label and success label displayed with URL */ | |
| export const CustomLabels: Story = { | |
| args: { | |
| text: 'https://example.com/invite/abc123', | |
| label: 'Copy link', | |
| successLabel: 'Link copied!', | |
| }, | |
| render: (args) => <CopyButtonWithLabel {...args} />, | |
| }; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/shared/copy-button.stories.tsx
Line: 42-132
Comment:
The three stories share identical wrapper markup. Extracting it into a local render helper cuts ~45 duplicate lines and makes per-story changes (e.g. different padding, dark background) trivial to apply consistently.
```suggestion
function CopyButtonWithLabel(args: React.ComponentPropsWithoutRef<typeof CopyButton>) {
return (
<div
style={{
display: 'flex',
flexDirection: 'column',
gap: 8,
alignItems: 'center',
}}
>
<code
style={{
fontSize: 13,
color: '#172033',
background: '#F4F6FA',
padding: '4px 8px',
borderRadius: 6,
}}
>
{args.text}
</code>
<CopyButton {...args} />
</div>
);
}
/** Default copy button with a URL displayed above */
export const Default: Story = {
args: {
text: 'https://example.com/invite/abc123',
},
render: (args) => <CopyButtonWithLabel {...args} />,
};
/** Short text content displayed above */
export const ShortText: Story = {
args: {
text: 'abc',
},
render: (args) => <CopyButtonWithLabel {...args} />,
};
/** Custom label and success label displayed with URL */
export const CustomLabels: Story = {
args: {
text: 'https://example.com/invite/abc123',
label: 'Copy link',
successLabel: 'Link copied!',
},
render: (args) => <CopyButtonWithLabel {...args} />,
};
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Contributor
Author
There was a problem hiding this comment.
✅ Addressed P2 — Copy-button wrapper duplication. Extracted 3 duplicate wrappers into a shared CopyButtonWithLabel render helper.
Contributor
|
Preview deployment ready: https://agent-comapeo-cloud-app-issu-7fbi.comapeo-cloud-app.pages.dev Commit: |
- Extract shared CopyButtonWithLabel wrapper in copy-button.stories.tsx - Restore .gitignore for storybook-screenshots-diff (prevent tracking diff PNGs) - Remove committed diff PNG artifact
Contributor
Author
|
✅ Addressed all review feedback:\n\n1. copy-button shared wrapper — Extracted duplicate wrapper markup into a shared |
Contributor
Author
|
✅ Updated screenshot baselines to match CI environment:
|
Contributor
Author
|
✅ CI is fully green! All checks passed including visual-regression, storybook tests, lint, typecheck, unit tests, and E2E tests. Changes in this PR:
🔁 Loop 2 — Requesting a fresh review. All previous feedback has been addressed and CI is green. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Automated implementation of #102.
Closes #102
Implemented by the agent implementation worker.
Greptile Summary
This PR refreshes Storybook screenshot baselines and resolves duplicate-baseline stories by making previously invisible UI states (popovers, tooltips, toasts) visible in static screenshots without user interaction.
BasemapSwitcher: AdefaultOpenprop (following React's uncontrolled-input naming convention) is added so theSatelliteSelectedstory can force the popover open, making the two stories visually distinct.open: true(Radix UI controlled prop) and toast stories use auseEffect-basedautoTriggerthat firesaddToastonce on mount;addToastis alreadyuseCallback-memoized with[]deps, so no duplicate toasts are created.CopyButtonWithLabelrender helper is extracted to display the copied text above the button, andSkeletonargs are moved from meta-level to theDefaultstory so each story can specify its own dimensions independently.Confidence Score: 5/5
Safe to merge — all changes are scoped to Storybook stories and screenshot baselines, with a minimal, convention-correct addition to BasemapSwitcher props.
The logic changes are limited to stories and test infrastructure. The one production-component change (defaultOpen on BasemapSwitcher) follows the standard React uncontrolled-input naming convention, initialises state correctly, and has no effect on existing callers that omit the prop.
No files require special attention.
Important Files Changed
defaultOpenprop (follows React uncontrolled-input convention) to allow Storybook stories to force the popover open for static screenshots; initializesuseState(defaultOpen)correctly.autoTriggerprop that firesaddToastin auseEffecton mount;addToastisuseCallback-memoized with[]deps, so the effect fires exactly once and no duplicate toasts are created.open: trueto all Tooltip stories so tooltips are visible in static screenshots; uses the Radix UI controlledopenprop correctly.CopyButtonWithLabelrender helper and wires it to all three stories, making the displayed text visible above the button for differentiated screenshots.*.pngto*, preventing any CI-generated artifact from accidentally being committed to the diff directory.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Storybook Screenshot CI] --> B{Story has\ninteractive UI?} B -- No --> C[Screenshot captured\nas-is] B -- Yes --> D{Fix strategy} D --> E[Tooltip\nopen: true\nRadix controlled prop] D --> F[Toast\nautoTrigger useEffect\nfires addToast on mount] D --> G[BasemapSwitcher\ndefaultOpen prop\ninitialises useState] E --> H[Visible in\nstatic screenshot] F --> H G --> H H --> I[Unique baseline\nper story]Comments Outside Diff (1)
tests/e2e/storybook-screenshots-diff/.gitignoreDeleting this
.gitignoreallows pixelmatch diff images to be committed, and one already is (desktop/components-mapcontainer--non-interactive.png). Diff images are CI-generated artifacts that go stale immediately after the next screenshot run, so tracking them in the repo will silently accumulate dead-weight binaries every time any baseline changes. If the intent is to make a single diff available for review, linking the CI artifact in the PR description achieves the same goal without touching the repo's history.Prompt To Fix With AI
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (3): Last reviewed commit: "chore: update storybook screenshot basel..." | Re-trigger Greptile