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

SignerPanel: adjust for new styles #920

Merged
merged 1 commit into from Aug 21, 2019
Merged

SignerPanel: adjust for new styles #920

merged 1 commit into from Aug 21, 2019

Conversation

sohkai
Copy link
Contributor

@sohkai sohkai commented Aug 19, 2019

Adjusts the signer panel to the new styles:

Screen Shot 2019-08-20 at 12 15 22 AM

Feedback statuses are implemented in #907.


Most of the other modes

Direct:

Screen Shot 2019-08-20 at 12 17 23 AM

Web3 not installed:

Screen Shot 2019-08-20 at 12 17 34 AM

Provider not enabled:

Screen Shot 2019-08-20 at 12 17 40 AM

Action impossible:

Screen Shot 2019-08-20 at 12 20 21 AM


TODO:

<ButtonLink onClick={onRequestEnable}>enable</ButtonLink>{' '}
<ButtonText
onClick={onRequestEnable}
horizontalPadding="none"
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should have horizontalPadding=none by default on ButtonText 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or provide an inline mode, since I find myself using it quite a lot in text :).

But yeah, I've almost always had to disable the horizontal padding when I need this component.

onClick={onRequestEnable}
horizontalPadding="none"
css={`
padding: 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

…or maybe we need two components: ButtonText would be without any padding, horizontal nor vertical, and Button would have a text mode, with a way to disable the padding on one or both sides.

Copy link
Contributor

@bpierre bpierre left a comment

Choose a reason for hiding this comment

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

👍 LGTM 👍

@sohkai sohkai changed the base branch from newstyle-permissions to newstyle August 21, 2019 22:51
@sohkai sohkai merged commit 24cd507 into newstyle Aug 21, 2019
@sohkai sohkai deleted the newstyle-signerpanel branch August 21, 2019 22:58
@dizzypaty
Copy link

All the Panel modes are looking great! ✨

For the case where we have multiple tx to sign, I've added to the 'Client' figma file a variation of the feedback module that is consistent with how we present multiple tx signing on the new on-boarding:

Group 132

Perhaps this is something that we can do after the launch, on another round of polishing.

@sohkai
Copy link
Contributor Author

sohkai commented Aug 27, 2019

@dizzypaty Let's do this separately, we would also have to handle the case where someone updates their organization through four transactions (if you had direct permissions to do so in the Kernel), and stacking four of those may look a bit odd.

2color added a commit that referenced this pull request Aug 28, 2019
…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)
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