Skip to content

feat(mobile-exp): Paginate through single screenshots in gallery#40260

Merged
shruthilayaj merged 13 commits into
masterfrom
shruthi/feat/paginate-single-screenshots
Oct 21, 2022
Merged

feat(mobile-exp): Paginate through single screenshots in gallery#40260
shruthilayaj merged 13 commits into
masterfrom
shruthi/feat/paginate-single-screenshots

Conversation

@shruthilayaj

@shruthilayaj shruthilayaj commented Oct 19, 2022

Copy link
Copy Markdown
Member

Paginate through single screenshots in gallery mode.
The pagination buttons are going to change based
on @olivier-w's designs, just want to get the functionality in.

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Oct 19, 2022
@github-actions

github-actions Bot commented Oct 19, 2022

Copy link
Copy Markdown
Contributor

size-limit report 📦

Path Size
src/sentry/static/sentry/dist/entrypoints/app.js 20.23 KB (-0.04% 🔽)
src/sentry/static/sentry/dist/entrypoints/sentry.css 33.06 KB (0%)

@shruthilayaj shruthilayaj marked this pull request as ready for review October 20, 2022 12:00
@shruthilayaj shruthilayaj requested a review from a team as a code owner October 20, 2022 12:00
@shruthilayaj shruthilayaj requested a review from a team October 20, 2022 12:00
Comment on lines +56 to +58
onClick={() => {
onCursor?.(links.next?.cursor, path, query, 1);
}}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering if it would be worthwhile to move these onClicks up and have an onPrevious and onNext, might be useful for analytics things but this might be a YAGNI situation.

I just felt like expecting the onCursor to be ready to accept a delta was too specific. I think you'd have to lift up some of the data to get path, query, and links but maybe we have those available in the parent already? Not sure. If there are any issues with this suggestion feel free to ignore it because I think you've clearly denoted this code fits a specific use-case for now

@shruthilayaj shruthilayaj merged commit 88630de into master Oct 21, 2022
@shruthilayaj shruthilayaj deleted the shruthi/feat/paginate-single-screenshots branch October 21, 2022 13:07
@github-actions github-actions Bot locked and limited conversation to collaborators Nov 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants