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

Settings: Garden permissions #860

Open
wants to merge 8 commits into
base: staging
Choose a base branch
from
Open

Settings: Garden permissions #860

wants to merge 8 commits into from

Conversation

rperez89
Copy link
Member

Still didn't added the filters since is going to take me some time given the way the connect returns the data for the roles but i can add later in another pr since i want to move to work on the conviction voting v2.

There is one case where the permission has no name for some reason, needs further investigation.

Screen Shot 2022-05-12 at 9 44 26 PM

@vercel
Copy link

vercel bot commented May 13, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
gardens-ui ✅ Ready (Inspect) Visit Preview May 13, 2022 at 0:48AM (UTC)

@@ -0,0 +1,118 @@
/* eslint-disable react/jsx-key */
Copy link
Member

Choose a reason for hiding this comment

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

Maybe the problem with names don't show up could be related to it. Same if it's not, I'll recommend remove it and set proper keys attributes to react render properly

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we really need this line also.

Copy link
Collaborator

@tmoutinho tmoutinho left a comment

Choose a reason for hiding this comment

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

Good job :fire. just a small comment.

import Permissions from './Permissions'


const LayoutWrapper = styled(Layout)`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice 🔥

Copy link
Member

Choose a reason for hiding this comment

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

Yeah loved that too

@@ -0,0 +1,118 @@
/* eslint-disable react/jsx-key */
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we really need this line also.

@@ -65,17 +90,18 @@ function GlobalPreferences({ compact, onClose, onNavigation, sectionIndex }) {
getEvmCrispr()
}, [account, connectedGarden, ethers, isSafari])

console.log('sectionIndex ', sectionIndex)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe remove the console log here

appAddress: string,
appName: string,
appIcon: string,
description: string | undefined, // TODO- check why this is happening proabably old ABI
Copy link
Member

Choose a reason for hiding this comment

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

You are still getting undefined here some times?

}

const rolesWithEntitiesResolved: Array<RoleProps> = appsWithRolesResolved ? appsWithRolesResolved.map((app: any) => {
return app.roles.map((role: any) => {
Copy link
Member

Choose a reason for hiding this comment

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

It's possible we get the correct type here in instead any, it could help track changes in the code below

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

Successfully merging this pull request may close these issues.

None yet

3 participants