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

Info boxes about locking panels (without actually doing it) #171

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

Conversation

mccalluc
Copy link
Contributor

@mccalluc mccalluc commented Nov 20, 2024

  • Towards Lock page to changes #88
  • Keep track of which panel we should be on, even if we clicked on a tab to look at another panel.
  • Explain why the user won't be allowed to touch these other pages
  • Wrap the message in a bootstrap info box.

There's enough here that I'd like sign off from @Shoeboxam and someone else before going farther.

For reviewers:

  • For both Mike and someone else: Do the warning messages express well why folks shouldn't be on the other tabs?
  • Is the styling ok? I'm not intending to spend a lot of time tweaking the UI, but do you think this is adequate? Maybe the boxes are too much, and bold text would suffice?

@mccalluc mccalluc changed the title Lock every panel except the current one Info boxes about locking panels (without actually doing it) Nov 21, 2024
@mccalluc mccalluc marked this pull request as ready for review November 21, 2024 12:44
Copy link
Member

@ekraffmiller ekraffmiller left a comment

Choose a reason for hiding this comment

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

I think the explanatory text is good, it describes well why the user shouldn't go back and tweak the inputs.
One comment on the locking behavior - could we make the lock only appear if the user has actually downloaded the release, as opposed to just viewing the download page?

I'm leaving this as a comment rather than approving, because @mccalluc mentioned that he would like @Shoeboxam's feedback on this.

@mccalluc
Copy link
Contributor Author

mccalluc commented Dec 2, 2024

could we make the lock only appear if the user has actually downloaded the release, as opposed to just viewing the download page?

Right now the final tab is just buttons, but my sense is that we may want to include a summary of the results on that page itself... perhaps the graphs? so the release would happen when you enter that tab by clicking the submit button on the previous tab. I think consistency in the locking behavior would be good: When you submit your selections on one page they are fixed, whether that is the first or the second tab.

@ekraffmiller
Copy link
Member

@mccalluc, ok if the release is going to appear on the page itself, I think that makes sense. Do you want me to go ahead and approve this or wait for comments from Mike?

@mccalluc
Copy link
Contributor Author

mccalluc commented Dec 3, 2024

@ekraffmiller, I'll wait for Mike's thoughts, so probably better to just to leave this as a comment so I don't merge it by mistake. Thanks for the feedback.

Comment on lines +112 to +114
Once you've confirmed your analysis settings
they are locked. The privacy budget should be considered
a finite resource.
Copy link
Member

@Shoeboxam Shoeboxam Dec 9, 2024

Choose a reason for hiding this comment

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

Upon further consideration, I've weakened the "we should lock once set" opinion I had, because I had a specific vision that may not be consistent with the intended use-case.

I think there are two modes:

  1. privacy unit and loss are configured externally and can never be changed
  2. privacy unit and loss are configured by analyst and can be changed, but with some cautions:

What to caution about:

  1. If there are existing releases, state that changing the privacy unit will re-run the release? Or maybe reset all stats? It should explain that changing the privacy unit is resetting the app in some way, or causing new releases to be run, and dropping the previous privacy spend. As we move forward, you (Chuck) may want to keep in mind that changing the input metric (from symmetric distance to user id distance, for example) could potentially make later queries invalid.
  2. The privacy unit has a significant impact on the integrity of the privacy guarantee. Please carefully consider if your new privacy unit is meaningful. While making your privacy unit more fine-grained can give the appearance of greater utility for the same privacy loss, it commensurately weakens the strength of the privacy guarantee.

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

Successfully merging this pull request may close these issues.

3 participants