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

Hide "person count" in person modal #5559

Closed
macobo opened this issue Aug 12, 2021 · 2 comments
Closed

Hide "person count" in person modal #5559

macobo opened this issue Aug 12, 2021 · 2 comments
Labels
enhancement New feature or request feature/persons Feature Tag: Persons

Comments

@macobo
Copy link
Contributor

macobo commented Aug 12, 2021

Is your feature request related to a problem?

We show a person count in our person modals:

image

I think it's problematic

  1. It causes confusion when the numbers don't line up
  2. We don't display the count beyond 100 anyways

Why would it cause confusion? Mainly due to caching. If you load a chart it's a good chance the chart is cached and a few hours old. But as we load persons, we load the most recent ones. So users inevitably wonder why the numbers don't match up when going from chart -> person modal.

Describe the solution you'd like

I propose we hide it.

Describe alternatives you've considered

Always caching persons together with the chart - but we're then doubling (or more) our load.

Additional context

cc @Twixes - His PR in #5537 made me think of this. Adding a "refresh" button may or may not be needed if we remove this.
cc @paolodamico - wdyt? Let's kill it?

Thank you for your feature request – we love each and every one!

@macobo macobo added enhancement New feature or request feature/persons Feature Tag: Persons labels Aug 12, 2021
macobo added a commit that referenced this issue Aug 12, 2021
@clarkus
Copy link
Contributor

clarkus commented Aug 12, 2021

I agree we should hide / remove it if the count isn't accurate.

paolodamico pushed a commit that referenced this issue Aug 12, 2021
@paolodamico
Copy link
Contributor

Fixed in #5560. We may want to consider bringing this back in the future when we figure out what to do about caching.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature/persons Feature Tag: Persons
Projects
None yet
Development

No branches or pull requests

3 participants