Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Update SelectionList indicators #4736

Merged
merged 3 commits into from
Mar 3, 2017
Merged

Update SelectionList indicators #4736

merged 3 commits into from
Mar 3, 2017

Conversation

jacogr
Copy link
Contributor

@jacogr jacogr commented Mar 3, 2017

parity 2017-03-03 13-50-30
parity 2017-03-03 13-50-03
parity 2017-03-03 13-49-40

@jacogr jacogr added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. B0-patch A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. A0-pleasereview 🤓 Pull request needs code review. labels Mar 3, 2017
@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Mar 3, 2017
@@ -58,7 +79,3 @@
filter: grayscale(10%);
opacity: 0.75;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we set it to 0.5? It would be more obvious IMO

Copy link
Contributor Author

@jacogr jacogr Mar 3, 2017

Choose a reason for hiding this comment

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

? Not sure it is for this PR?

  1. As-is in master currently?
  2. Remember the original complaint, it looked "disabled" so users didn't know what to do. (It came from exactly that value)

Agreed though, I'm "ok" but not "happy" with the selections, warrants a bit more playing with the colors, borders & opacity.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well it's somehow related to this PR since it removes the tick, thus makes non-selected accounts less obvious : that's why I'm suggesting an opacity of 0.5... After all there are no such things as disabled accounts, so I'm not sure if that's a real issue ?

Copy link
Contributor Author

@jacogr jacogr Mar 3, 2017

Choose a reason for hiding this comment

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

There are disabled accounts. Hardware wallets show up as disabled/inactive when they are not plugged in. (Same on the signer - when not in it is disabled/inactive. Those are only in the list & signer views though)

My preference for the borders (and actual icon colours) would be the "almost yellow" as in the CreateAccount dialog icons. But that needs some playing. (Basically the opposite of the link blue)

@ngotchac ngotchac added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. A0-pleasereview 🤓 Pull request needs code review. A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A0-pleasereview 🤓 Pull request needs code review. A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Mar 3, 2017
@gavofyork gavofyork merged commit cb118f1 into master Mar 3, 2017
@gavofyork gavofyork deleted the jg-selectors branch March 3, 2017 18:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants