Skip to content

Feature: Bookmark Categories #6035

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

Merged
merged 8 commits into from
Dec 17, 2024

Conversation

parneet-guraya
Copy link
Contributor

Fixes #5922

What changes did you make and why?

  • Add bookmark button in bookmark details screen.
  • Add new bookmark tab in bookmarks screen.

Tests performed (required)

Tested betaDebug on Oneplus 9RT 5G (Android 14)

Screenshots (for UI changes only)

Record_2024-12-16-01-38-04.mp4

@parneet-guraya
Copy link
Contributor Author

parneet-guraya commented Dec 15, 2024

@nicolas-raoul

Hi, I have few questions,

  • When logged out we only show bookmark pictures, do we want to show categories too or not when logged out?

  • Right now we're loading the entire data. But, do we want to load the items lazily (paging) ? Other bookmarks types (picture, items, locations) does not use paging either.

Obviously that depends on the size of data but do we expect the bookmarks to be in large amounts?

  • For the whole bookmark feature (every type) should we have search, sort?

Thanks :)

@parneet-guraya parneet-guraya marked this pull request as draft December 15, 2024 20:45
@parneet-guraya parneet-guraya marked this pull request as ready for review December 15, 2024 21:20
@nicolas-raoul
Copy link
Member

Show when logged out: yes

Paging, sorting, searching: Great idea, though the added complexity would currently not be justified. Would you mind creating a new issue and labeling it as enhancement, low priority? Thanks a lot! 🙂

@parneet-guraya
Copy link
Contributor Author

Show when logged out: yes

Great! I also kept it that way.

Paging, sorting, searching: Great idea, though the added complexity would currently not be justified. Would you mind creating a new issue and labeling it as enhancement, low priority? Thanks a lot! 🙂

I will shortly, outside right now (commenting from my mobile). :)

Signed-off-by: parneet-guraya <gurayaparneet@gmail.com>
Signed-off-by: parneet-guraya <gurayaparneet@gmail.com>
Signed-off-by: parneet-guraya <gurayaparneet@gmail.com>
Signed-off-by: parneet-guraya <gurayaparneet@gmail.com>
Signed-off-by: parneet-guraya <gurayaparneet@gmail.com>
Signed-off-by: parneet-guraya <gurayaparneet@gmail.com>
Signed-off-by: parneet-guraya <gurayaparneet@gmail.com>
Signed-off-by: parneet-guraya <gurayaparneet@gmail.com>
@parneet-guraya
Copy link
Contributor Author

Docs added :)

Copy link
Member

@nicolas-raoul nicolas-raoul left a comment

Choose a reason for hiding this comment

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

Working great, thanks a lot!

@nicolas-raoul nicolas-raoul merged commit 5500b03 into commons-app:main Dec 17, 2024
1 check passed
@parneet-guraya parneet-guraya deleted the bookmark-categories branch December 17, 2024 12:49
@parneet-guraya
Copy link
Contributor Author

@nicolas-raoul This closes this #5318 issue as well. Take a look :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bookmarking categories
2 participants