Skip to content
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

feat(web): revamp places #12219

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

kvalev
Copy link

@kvalev kvalev commented Sep 1, 2024

Description

The Places page has been slightly revamped as follows:

  • Added a counter for number of places
  • Added a search box for filtering places
  • Added places grouping: default is no grouping, but the cities can be grouped by country (similarly to how the albums are grouped by year). When searching for places, empty groups will be hidden. If places are grouped, then icons for expanding or collapsing all will be shown.

Open points/questions:

  • currently the country/group counter says N Albums instead of N Places, but I havent figured out the translations yet
  • part of the grouping dropdown is outside the screen when no grouping has been selected
  • translations?!?
  • tests?!?

Screenshots

Default view places-default-view
Grouped (looks a bit dumb on the demo server, as there is usually only 1 place per country) places-grouped
Grouped and collapsed places-collapsed
Grouped and filtered places-filtered

fixes #2631

@alextran1502
Copy link
Contributor

image

Thank you for the PR, looks great! What is the count here? I assume it means one place for the country, not an album, correct?

@kvalev
Copy link
Author

kvalev commented Sep 1, 2024

Thank you for the PR, looks great! What is the count here? I assume it means one place for the country, not an album, correct?

Yeah exactly, this should be the number of places for the given country, but currently it says albums, as I havent figured out the translations, so I reused the one from the albums page :D

@kvalev
Copy link
Author

kvalev commented Sep 1, 2024

The label should be correct now, at least in English, I havent added any translations:

places-fixed-translations

@alextran1502
Copy link
Contributor

Hello, I run into this issue while trying to change the grouping

image

@zackpollard
Copy link
Contributor

Closing as no response to feedback, please feel free to re-open a new PR or comment on this one if you are planning to continue work on this in the future! 😄

@kvalev
Copy link
Author

kvalev commented Dec 19, 2024

👋 I am back from traveling and can pick up where I left off. Would you mind reopening the PR?

@jrasm91 jrasm91 reopened this Dec 19, 2024
@kvalev kvalev force-pushed the kvv-revamp-places-page branch 2 times, most recently from 2899b66 to 0c777aa Compare January 26, 2025 01:54
@kvalev kvalev force-pushed the kvv-revamp-places-page branch from 0c777aa to a664bd6 Compare January 26, 2025 09:39
@kvalev
Copy link
Author

kvalev commented Jan 26, 2025

I rebased the PR and checked that there are no errors in the console. Can you take another look? I cant figure out how to fix the layout problem when no grouping is selected, but I am hoping one of you has an idea.

@alextran1502 alextran1502 self-assigned this Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

4 participants