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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 0 additions & 25 deletions js/src/modals/DappPermissions/dappPermissions.css

This file was deleted.

15 changes: 0 additions & 15 deletions js/src/modals/DappPermissions/dappPermissions.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ import { FormattedMessage } from 'react-intl';
import { connect } from 'react-redux';

import { AccountCard, Portal, SelectionList } from '~/ui';
import { CheckIcon, StarIcon } from '~/ui/Icons';

import styles from './dappPermissions.css';

@observer
class DappPermissions extends Component {
Expand All @@ -40,18 +37,6 @@ class DappPermissions extends Component {

return (
<Portal
buttons={
<div className={ styles.legend }>
<FormattedMessage
id='dapps.permissions.description'
defaultMessage='{activeIcon} account is available to application, {defaultIcon} account is the default account'
values={ {
activeIcon: <CheckIcon />,
defaultIcon: <StarIcon />
} }
/>
</div>
}
onClose={ permissionStore.closeModal }
open
title={
Expand Down
29 changes: 23 additions & 6 deletions js/src/ui/SelectionList/selectionList.css
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,29 @@

.overlay {
position: absolute;
right: 0.5em;
top: 0.5em;
left: 0.75em;
top: 0.75em;
z-index: 1;

.icon,
.iconDisabled {
border-radius: 0.25em;
color: #333 !important;
cursor: pointer;
margin-right: 0.25em;
}

.icon {
background: white;
}

.iconDisabled {
background: #666;

&:hover {
background: white;
}
}
}
}

Expand All @@ -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)

}

.iconDisabled {
opacity: 0.15;
}
23 changes: 11 additions & 12 deletions js/src/ui/SelectionList/selectionList.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import React, { Component, PropTypes } from 'react';

import { CheckIcon, StarIcon, StarOutlineIcon } from '~/ui/Icons';
import { StarIcon } from '~/ui/Icons';
import SectionList from '~/ui/SectionList';
import { arrayOrObjectProptype } from '~/util/proptypes';

Expand Down Expand Up @@ -62,9 +62,15 @@ export default class SelectionList extends Component {
let defaultIcon = null;

if (onDefaultClick) {
defaultIcon = isSelected && item.default
? <StarIcon />
: <StarOutlineIcon className={ styles.iconDisabled } onClick={ makeDefault } />;
defaultIcon = (
<div className={ styles.overlay }>
{
isSelected && item.default
? <StarIcon className={ styles.icon } />
: <StarIcon className={ styles.iconDisabled } onClick={ makeDefault } />
}
</div>
);
}

const classes = isSelected
Expand All @@ -83,14 +89,7 @@ export default class SelectionList extends Component {
>
{ renderItem(item, index) }
</div>
<div className={ styles.overlay }>
{ defaultIcon }
{
isSelected
? <CheckIcon onClick={ selectItem } />
: <CheckIcon className={ styles.iconDisabled } onClick={ selectItem } />
}
</div>
{ defaultIcon }
</div>
);
}
Expand Down