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

feat(smart-slippage): update smart slippage text #4982

Merged
merged 7 commits into from
Oct 14, 2024

Conversation

alfetopito
Copy link
Collaborator

Summary

Follow up to #4934, addressing these comments #4934 (comment) from @MindyCoW

Additionally, makes sure the High slippage amount selected in the settings tab is only displayed if user picks a value above the suggested slippage - if any. Otherwise sticks to the default of 1%.

To Test

  1. Check the texts with and without smart slippage selected match feat(slippage): small order slippage v2 #4934 (comment)
  • Should match
  1. In the settings modal, set a slippage amount higher than the suggested slippage
  • Should show High slippage amount selected
  1. When suggested amount is > 1%, set a slippage amount > 1% and smaller than suggested slippage
  • Should not show High slippage amount selected
  1. Disable smart slippage flags
  • Should work as before

@alfetopito alfetopito self-assigned this Oct 11, 2024
Copy link

vercel bot commented Oct 11, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
cosmos ✅ Ready (Inspect) Visit Preview Oct 14, 2024 3:06pm
cowfi ✅ Ready (Inspect) Visit Preview Oct 14, 2024 3:06pm
explorer-dev ✅ Ready (Inspect) Visit Preview Oct 14, 2024 3:06pm
swap-dev ✅ Ready (Inspect) Visit Preview Oct 14, 2024 3:06pm
widget-configurator ✅ Ready (Inspect) Visit Preview Oct 14, 2024 3:06pm

@elena-zh
Copy link
Contributor

Hey @alfetopito , great!

Some notes:

  1. I think the slippage tooltip should be the same on the form/settings/confirm modal. Right?
    same 2
    same
  2. (nitpick!) but we don't have gear icon for settings anu more! Gear icon is used for a theme switch
    not gear
  3. Is this OK that in a mobile view 'recommended' is shifted to another line?
    another line

Thanks!

@shoom3301
Copy link
Collaborator

@alfetopito there are several TODOs in SmartSlippageUpdater, please, don't forget about them

@alfetopito
Copy link
Collaborator Author

Hey @alfetopito , great!

Some notes:

1. I think the slippage tooltip should be the same on the form/settings/confirm modal. Right?
   ![same 2](https://private-user-images.githubusercontent.com/70885163/375818806-bef94c40-807f-4b9a-a417-84f096c1f349.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mjg2NjMwNjAsIm5iZiI6MTcyODY2Mjc2MCwicGF0aCI6Ii83MDg4NTE2My8zNzU4MTg4MDYtYmVmOTRjNDAtODA3Zi00YjlhLWE0MTctODRmMDk2YzFmMzQ5LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDEwMTElMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMDExVDE2MDYwMFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTNhNDMzNzdmZjk0NTM4YzM1ZjM4ZjZkNWM4MzliZWUzMzU1YzRkYTJhMWMwMjYxYzNjYTg3MzhmZTJmOTVkNjYmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.FY9ig2nBfHGX-TEy9LYVvwqFj6iqRjGN4VioUdRMGGs)
   ![same](https://private-user-images.githubusercontent.com/70885163/375818817-cf99eedd-c4ba-4ea0-ba26-a4882950de08.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mjg2NjMwNjAsIm5iZiI6MTcyODY2Mjc2MCwicGF0aCI6Ii83MDg4NTE2My8zNzU4MTg4MTctY2Y5OWVlZGQtYzRiYS00ZWEwLWJhMjYtYTQ4ODI5NTBkZTA4LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDEwMTElMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMDExVDE2MDYwMFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTNkNGZmM2IzYTc4YmYzNjg2Mjk3MzM2Y2M0YzhkYmVjNzU1NzFkMWY2MTcxZDY3YzQyYWZhYTVlZGM2ZGY4MDEmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.jh-n5_tYUsZAZPfjpqxJqB3xapjf6wgqVQE8lDUc6jo)

Fixed

2. (nitpick!) but we don't have gear icon for settings anu more! Gear icon is used for a theme switch
   ![not gear](https://private-user-images.githubusercontent.com/70885163/375819236-38b83f51-0192-4500-bbf3-c0a1d57c7487.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mjg2NjMwNjAsIm5iZiI6MTcyODY2Mjc2MCwicGF0aCI6Ii83MDg4NTE2My8zNzU4MTkyMzYtMzhiODNmNTEtMDE5Mi00NTAwLWJiZjMtYzBhMWQ1N2M3NDg3LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDEwMTElMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMDExVDE2MDYwMFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWQyMDBlMTVmZDc4Yzc4NDlmZjU0NjcxNTEyMmE1NDUyZTlkYTk0MjUyZjk4YzQxYTQ0YWNhMDI0NjdhMWU2YmMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.j8jAeM_lSp5NezCSvGqHbJtiwi9KjZ0v9vHmo8Y0oAw)

Fixed

3. Is this OK that in a mobile view 'recommended' is shifted to another line?
   ![another line](https://private-user-images.githubusercontent.com/70885163/375819633-01b8104b-5ff8-45fb-91c6-68be68f94076.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mjg2NjMwNjAsIm5iZiI6MTcyODY2Mjc2MCwicGF0aCI6Ii83MDg4NTE2My8zNzU4MTk2MzMtMDFiODEwNGItNWZmOC00NWZiLTkxYzYtNjhiZTY4Zjk0MDc2LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDEwMTElMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMDExVDE2MDYwMFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTk1OTlmY2JlMTE5NzRiNzhmNGEwNDA4YTljMjMyYmZiNjI4YjE0ZTVhZmUzNjRhYzYwOGE0MTljOTdmNGFjYzgmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.1yQQb1okuw7m__9sWONZQWdtA0NqOlxA7K2o6amDs2s)

Thanks!

I tried but failed to fix the CSS issue :(
Will have to ask @fairlighteth

Copy link
Contributor

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

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

@alfetopito , thanks! I've opened #4985 for @fairlighteth to address this issue.

@alfetopito alfetopito merged commit 4b89ecb into develop Oct 14, 2024
11 of 12 checks passed
@alfetopito alfetopito deleted the feat/small-order-slippage-texts branch October 14, 2024 16:57
@github-actions github-actions bot locked and limited conversation to collaborators Oct 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants