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
Add entity display comps #222
Conversation
|
||
const MetaSection = () => { | ||
const { entity } = useEntityContext(); | ||
const application = entity?.item as Application; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could pull out all the fields here and give sensible defaults:
const {id, name, ... } = entity?.item
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stylistic choice for you. if application might have a lot of undefined values makes more sense to keep it as is and have the individual sections handle their undefined
states
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll likely move towards this kinda thing when i get the EntityContext more fleshed out. Will keep in mind!
@@ -1,12 +1,15 @@ | |||
import { ResourceType } from 'common/enums'; | |||
import UserContentPanel from './UserContentPanel'; | |||
import GroupContentPanel from './GroupContentPanel'; | |||
import ApplicationContentPanel from './ApplicationContentPanel'; | |||
import PolicyContentPanel from './PolicyContentPanel'; | |||
|
|||
// convert to ParentResource? | |||
const PanelComponents = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small suggestion because I know there's a lot of upgrading old code going on.
for objects like this that you're never mutating, you can use Object.freeze() to ensure they never change
@@ -2,7 +2,7 @@ | |||
import { css } from '@emotion/react'; | |||
import styled from '@emotion/styled'; | |||
import { ReactNode } from 'react'; | |||
import { Grid } from 'semantic-ui-react'; | |||
import { Grid, SemanticWIDTHS } from 'semantic-ui-react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SemanticWIDTHS
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks really odd, just making sure it's not a typo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's what vscode pulled out of the semantic types... 😛
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good. if you wanted to you could probably pull out ContentPanel
into reusable component as its "made" 3 times.
const ContentPanel = (controls, section, errorMsg) => <...>`
Yeah was thinking about that! but will prob do in the next pr after this |
Adds display components for Group, Application and Policy metadata
Minor styling adjustments
Content
mode
is mocked toDISPLAYING
for nowPart of #200