-
Notifications
You must be signed in to change notification settings - Fork 273
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
Home redesign #934
Home redesign #934
Conversation
bpierre
commented
Aug 24, 2019
onMessage={this.handleAppMessage} | ||
onOpenApp={this.openApp} | ||
/> | ||
<AppInternal> |
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.
Wonder if it would make sense to wrap Home within its own file? What do you think?
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 mean in a different way than it is now? https://github.com/aragon/aragon/pull/934/files#diff-cdace88e2d25deba8b1d9ed1f6560baf
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.
Just wondering if it could be:
export default React.memo(props => <AppInternal><Home {..props} /></AppInternal>)
To avoid more code in Wrapper
, no other reason.
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.
Oh I see, yes good idea! We are probably going to wrap every app in OrgView
with Root
, so maybe we can do this + move all the AppInternal
inside the apps at the same time 👍
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.
😍
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.
LGTM 💥🚀
…tions * origin/newstyle: MenuPanel tweaks (#933) Home redesign (#934) SignerPanel: consolidate external transaction props into intent object (#931) useLocalIdentity: handle remove case (#930) Add AppInternal to manage the layout logic of internal apps (#932) MenuPanel: adjust for new styles (#923) SignerPanel: display warning for external transactions (#850) Remove Badge and update occurrences for Tag (#901) SignerPanel: adjust for new styles (#920) Organization Settings: replace old Settings app (#896) Permissions: new style (#899) Sidepanel: redesign feedback indicator (#907) eslint: make sure curly braces are used everywhere (#924) Org switcher: new style + add FavoritesMenu (#925)