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

Add scrolling button to account list #5957

Closed
wants to merge 1 commit into from

Conversation

alextsg
Copy link
Contributor

@alextsg alextsg commented Dec 20, 2018

Fixes #5140

@cjeria I'm using a placeholder arrow icon that we currently have and just rotated it, but it doesn't exactly match what's in the mock. Once I get the icon from you I'll update this PR.

scroll-accounts

@alextsg alextsg added the DO-NOT-MERGE Pull requests that should not be merged label Dec 20, 2018
renderKeyringType (keyring) {
const { t } = this.context

try { // Sometimes keyrings aren't loaded yet:
Copy link
Contributor

Choose a reason for hiding this comment

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

In the case where keyrings aren't loaded, what exception are we catching here? Can we instead have a default for type or return null if !keyring.type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not really sure what the try catch is for as I just copied this over from the original code, but yeah, we should be able to remove this.

@cjeria
Copy link
Contributor

cjeria commented Jan 2, 2019

@alextsg looks great! I've slacked you the SVG icon, and also including a direct link to download https://www.dropbox.com/s/j1swxdzxacpowb2/down_arrow.svg?dl=0

@alextsg alextsg force-pushed the i5140-accounts branch 2 times, most recently from 884d36e to 0d1e4c5 Compare January 3, 2019 19:11
@alextsg alextsg closed this Jan 3, 2019
@alextsg alextsg deleted the i5140-accounts branch January 3, 2019 19:23
@alextsg alextsg restored the i5140-accounts branch January 3, 2019 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO-NOT-MERGE Pull requests that should not be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants