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

Allow list being a requirement is not clear in the UI #9746

Open
timolegros opened this issue Oct 31, 2024 · 6 comments · May be fixed by #9903
Open

Allow list being a requirement is not clear in the UI #9746

timolegros opened this issue Oct 31, 2024 · 6 comments · May be fixed by #9903
Assignees
Labels
1 1-2 hour task bug Something isn't working

Comments

@timolegros
Copy link
Collaborator

Description

#9690 (comment)

We need to clarify that the allowlist is part of the requirements when deciding on the minimum number of requirements that should be met.

For example in this picture:
image
It is unclear that the allowlist is a requirement and that a user needs to either meet the token threshold or be in the allowlist.

Same here:
image

Additional context

This caused "bugs" like #9690.

@timolegros
Copy link
Collaborator Author

Optionally, the allowlist should not be considered a requirement. The reason being that it is impossible to create the following gating rule: meet all of the token threshold requirements OR be in the allowlist.

The downside with excluding the allowlist from the requirements is that you cannot apply token threshold requirements to allow listed users i.e. you can't do: meet any number of token threshold requirements AND be in the allowlist.

It seems to me that the allowlist is better utilized as a whitelist like in the OR case where token threshold requirements never apply to whitelisted members but growth/product would know best here.

@Israellund Israellund self-assigned this Nov 4, 2024
@sachben91
Copy link

agree with @timolegros here that the Allowlist should not be considered a requirement, probably need to be communicated better in the UI. @sssssabinaaa likely need your input here

@timolegros
Copy link
Collaborator Author

agree with @timolegros here that the Allowlist should not be considered a requirement, probably need to be communicated better in the UI. @sssssabinaaa likely need your input here

If the allowlist should not be considered part of the requirements then I don't think we need any UI changes. In fact the copy above the allow list settings already indicates that it is not part of the requirements (we could maybe update "conditions' to "requirements" to make it more clear).

@sssssabinaaa
Copy link
Contributor

to reduce confusion the "necessary requirements" section should be above the section with the ERC token selection

@timolegros
Copy link
Collaborator Author

Aight so @Israellund this is a 2fer:

  1. UI change
  2. Update gating func on backend to exclude allow list from number of requirements check.

@Israellund
Copy link
Collaborator

@timolegros @sssssabinaaa @sachben91 🫡

@Israellund Israellund added 1 1-2 hour task and removed needs estimate labels Nov 6, 2024
@Israellund Israellund linked a pull request Nov 14, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 1-2 hour task bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants