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

Voting: add apps filter #967

Merged
merged 5 commits into from
Sep 4, 2019
Merged

Voting: add apps filter #967

merged 5 commits into from
Sep 4, 2019

Conversation

sohkai
Copy link
Contributor

@sohkai sohkai commented Sep 3, 2019

Adds the apps filters to Voting. Requires a new release of all aragonAPI libraries.

Screen Shot 2019-09-03 at 6 14 59 PM

@dizzypaty I've added two mandatory options to this list:

  • Voting (this): selects votes without a script (i.e. polls)
  • External: selects votes to target contracts outside of the current organization

The dropdown list is sorted alphabetically by name. None are currently loaded, but identifiers (no custom labels yet) will be shown as, e.g. Tokens (ANT).

I was thinking it'd be nice to have external, but it's unlikely someone would be using the Voting app to make external calls at this point (they'd likely just be making interactions against an installed Agent app), so we can remove it.

@coveralls
Copy link

coveralls commented Sep 3, 2019

Coverage Status

Coverage decreased (-0.3%) to 97.635% when pulling c076fca on voting-filter-apps into d3cf90c on master.

@dizzypaty
Copy link

dizzypaty commented Sep 3, 2019

@sohkai re your comments:

  • I think Voting (this) is a bit confusing, specially in the case of having multiple voting apps installed. You think it's not right to leave it just as 'Voting'? If not, how about Voting (polls) or Voting (questions)?
  • It's nice to include a filter for External calls / contracts.

I don't see the filter header in that image, so just to be on the safe side: it should say 'Apps' on the top of the panel using text-style label2.

@sohkai
Copy link
Contributor Author

sohkai commented Sep 3, 2019

I think Voting (this) is a bit confusing, specially in the case of having multiple voting apps installed. You think it's not right to leave it just as 'Voting'? If not, how about Voting (polls) or Voting (questions)?

Yeah, I'm not sure what the most clear way of expressing this will be. On an org with multiple voting apps (e.g. governance), having

Voting
Voting (50% 50%)

Would be quite confusing. This app doesn't really make much sense either.

@dizzypaty
Copy link

dizzypaty commented Sep 4, 2019

Discussed offline for filter options: in the case of multiple app instances, the filter items should display the custom label. If custom labels are not available, we'll use tagIndicator to communicate which app instance users are currently interacting with. (Figma file)

VOTING

@sohkai
Copy link
Contributor Author

sohkai commented Sep 4, 2019

@dizzypaty I'll work on the other dropdown-related issues in their own issue.

This is how the dropdown current looks like (imagine there are multiple apps installed for Voting and Tokens):

Screen Shot 2019-09-04 at 10 59 06 PM


For now let's not add logic for detecting multiple app instances and refresh this in its own iteration afterwards (with custom label resolution too). The "this app" tag and identifiers in parenthesis will always appear in the filter.

Added 15e714f to detect multiple app instances before displaying identifiers.

@sohkai sohkai merged commit edc7121 into master Sep 4, 2019
@sohkai sohkai deleted the voting-filter-apps branch September 4, 2019 21:43
ramilexe pushed a commit to ConsiderItDone/aragon-apps that referenced this pull request Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants