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.

Sign up for GitHub

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

Jump to bottom

Select account - unified account display #995

Merged
merged 11 commits into from
Nov 7, 2023
Merged

Select account - unified account display #995

merged 11 commits into from
Nov 7, 2023

Conversation

bobo-k2
Copy link
Contributor

@bobo-k2 bobo-k2 commented Oct 24, 2023

Pull Request Summary

Clearly display unified account in the account selection modal
image

Check list

  • contains breaking changes
  • adds new feature
  • modifies existing feature (bug fix or improvements)
  • relies on other tasks
  • documentation changes
  • tested on mobile devices

@github-actions
Copy link

github-actions bot commented Oct 24, 2023 •
edited
Loading

Visit the preview URL for this PR (updated for commit 75df611):

https://astar-apps--pr995-feat-au-select-accou-uv7hr1ic.web.app

(expires Mon, 06 Nov 2023 09:40:40 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: dd76fe72958fe2910fef9d53f0b4539b82b849db

@Kahonnohak
Copy link
Contributor

Looks good! Now we know the account a lot better:)
Could you move the balance on the right to save some space?

Screenshot 2023-10-26 at 09 19 19

@bobo-k2
Copy link
Contributor Author

bobo-k2 commented Oct 26, 2023

@Kahonnohak I repositioned amount, but now it has conflict with green dot which marks current account. IMO we should remove this dot and simply select radio box on the left if an account is currently connected. Other option would be to reposition the dot, but not sure where.
image

@gluneau
Copy link
Contributor

gluneau commented Oct 26, 2023

We may need a dark theme adjustment...

image

@bobo-k2
Copy link
Contributor Author

bobo-k2 commented Oct 27, 2023

We may need a dark theme adjustment...

Thanks Greg. Fixed like this
image

impelcrypto
impelcrypto approved these changes Oct 30, 2023
View reviewed changes
Copy link
Member

@impelcrypto impelcrypto left a comment

Choose a reason for hiding this comment

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

LGTM

Maybe adjust the styling to make some space between the balance and dot?
image

@bobo-k2
Copy link
Contributor Author

bobo-k2 commented Oct 30, 2023

LGTM

Maybe adjust the styling to make some space between the balance and dot?

Fixed here 75df611
image

gluneau
gluneau approved these changes Oct 30, 2023
View reviewed changes
Copy link
Contributor

@gluneau gluneau left a comment

Choose a reason for hiding this comment

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

LGTM

@Kahonnohak
Copy link
Contributor

We could remove that dot if we don't need it - I didn't particular find it useful.

Thanks LGTM

@bobo-k2 bobo-k2 merged commit aaee0cc into main Nov 7, 2023
6 checks passed
@bobo-k2 bobo-k2 deleted the feat/au-select-account branch November 7, 2023 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gluneau gluneau gluneau approved these changes

@Kahonnohak Kahonnohak Kahonnohak approved these changes

@impelcrypto impelcrypto impelcrypto approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bobo-k2 @Kahonnohak @gluneau @impelcrypto