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

chore: pending review feedback for token details related changes #6530

Merged
merged 6 commits into from
Jun 8, 2023
Merged

chore: pending review feedback for token details related changes #6530

merged 6 commits into from
Jun 8, 2023

Conversation

nikoferro
Copy link
Contributor

Changes included for asset overview view

  • Avoid trying to convert the price to selected currency if there is no fiat price reference available
  • Add matching gradient to the area chart
  • Make chart size bigger in relation to the rest of the asset overview
  • Better gesture handling when interacting with the chart
  • Subtle shadow on asset overview footer to match other parts of the app
  • Change name from ETHER to Ethereum for mainnet's native currency

Screenshots/Recordings

screen

@nikoferro nikoferro requested a review from a team as a code owner June 5, 2023 13:58
@nikoferro nikoferro mentioned this pull request Jun 5, 2023
Closed
@nikoferro nikoferro added needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) release-7.1.0 Issue or pull request that will be included in release 7.1.0 labels Jun 5, 2023
@nikoferro nikoferro requested review from leotm and jpcloureiro June 5, 2023 14:00
leotm
leotm previously approved these changes Jun 5, 2023
View reviewed changes
@davibroc davibroc added the QA Passed A successful QA run through has been done label Jun 7, 2023
@davibroc
Copy link
Contributor

davibroc commented Jun 7, 2023

I have tested this and marked this QA Passed

davibroc
davibroc previously approved these changes Jun 7, 2023
View reviewed changes
@cortisiko cortisiko removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Jun 7, 2023
@sethkfman sethkfman mentioned this pull request Jun 7, 2023
Merged
3 tasks
@nikoferro nikoferro dismissed stale reviews from davibroc and leotm via 8443d8e June 8, 2023 09:08
@nikoferro nikoferro requested review from leotm and davibroc June 8, 2023 09:12
leotm
leotm approved these changes Jun 8, 2023
View reviewed changes
Copy link
Member

@leotm leotm left a comment

Choose a reason for hiding this comment

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

just re-reviewed changes, lgtm

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 8, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

10.7% 10.7% Coverage
0.0% 0.0% Duplication

@nikoferro nikoferro merged commit d192935 into main Jun 8, 2023
@nikoferro nikoferro deleted the swaps/token-details-feedback-v2 branch June 8, 2023 09:43
@github-actions github-actions bot locked and limited conversation to collaborators Jun 8, 2023
@cortisiko cortisiko added release-7.1.0 Issue or pull request that will be included in release 7.1.0 and removed release-7.1.0 Issue or pull request that will be included in release 7.1.0 labels Jun 13, 2023
@cortisiko
Copy link
Member

was cherry picked into 7.1: #6578

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@davibroc davibroc davibroc approved these changes

@leotm leotm leotm approved these changes

@jpcloureiro jpcloureiro Awaiting requested review from jpcloureiro

Assignees
No one assigned
Labels
QA Passed A successful QA run through has been done release-7.1.0 Issue or pull request that will be included in release 7.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nikoferro @davibroc @cortisiko @leotm