-
Notifications
You must be signed in to change notification settings - Fork 4
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
Update modal styles on mobile #898
Conversation
✅ Deploy Preview for acre-dapp-testnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for acre-dapp ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for acre-dapp-testnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for acre-dapp ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall it looks good but I have a few thoughts:
- Let's not change padding in modal windows for standalone dApp.
- Some time ago we decided to show modal windows on top for mobile devices. More information here.
- I have tested the smallest window for Ledger Live desktop and it seems to me that we should not change the padding for such a window size yet. What do you think? 🤔
There is no need to change the modal width - we should use the previous value.
The `ModalHeader` and `ModalBoyd` components should have the same horizontal padding.
Closes: AENG-36
Closes: AENG-37
This PR updates the modal component styles on mobile devices.