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

Sidepanel feedback #907

Merged
merged 6 commits into from
Aug 21, 2019
Merged

Sidepanel feedback #907

merged 6 commits into from
Aug 21, 2019

Conversation

AquiGorka
Copy link
Contributor

@AquiGorka AquiGorka commented Aug 14, 2019

Accompanying changes for Voting (although these apply everywhere)

@sohkai sohkai requested a review from bpierre August 14, 2019 15:42
Copy link
Contributor

@sohkai sohkai left a comment

Choose a reason for hiding this comment

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

Looks good, a couple small things to decide :).

src/components/SignerPanel/SigningStatus.js Outdated Show resolved Hide resolved
src/components/SignerPanel/SigningStatus.js Show resolved Hide resolved
src/components/SignerPanel/SigningStatus.js Outdated Show resolved Hide resolved
justify-content: center;
width: 60px;
height: 60px;
border: 2px solid ${color};
Copy link
Contributor

Choose a reason for hiding this comment

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

We should increase the border when it's successful or had an error (designs have them at 3px but we may want to play around with this).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I recall correctly this was updated to 40px width and height and 3px border right?

Copy link
Contributor

@sohkai sohkai Aug 19, 2019

Choose a reason for hiding this comment

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

We have to check with @dizzypaty on this; that was only for the Voting app but may not be the case for the signing panel itself (since its feedback surface is much larger).

Choose a reason for hiding this comment

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

Yes, it was updated in the Voting app module (to be 40x40px) to make it more compact for that context. But we should keep the current size (60x60px) on the signer panel. As Brett said, the reason being its surface being larger here and being the most important item the users need to look at when performing this action. Once we standarize this component, it will make sense to define all its possible sizes depending on where is presented.

@sohkai sohkai mentioned this pull request Aug 15, 2019
22 tasks
@vercel vercel bot temporarily deployed to staging August 19, 2019 12:02 Inactive
@vercel vercel bot temporarily deployed to staging August 19, 2019 12:02 Inactive
@sohkai sohkai mentioned this pull request Aug 19, 2019
1 task
@vercel vercel bot temporarily deployed to staging August 21, 2019 19:27 Inactive
@vercel vercel bot temporarily deployed to staging August 21, 2019 19:30 Inactive
@sohkai sohkai merged commit 714c1e6 into newstyle Aug 21, 2019
@sohkai sohkai deleted the newstyle-0.8/sidepanel branch August 21, 2019 19:40
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.

4 participants