Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Close button is not aligned with text in payment history modal #5719

Closed
srirambv opened this issue Nov 17, 2016 · 13 comments · Fixed by #6047
Closed

Close button is not aligned with text in payment history modal #5719

srirambv opened this issue Nov 17, 2016 · 13 comments · Fixed by #6047
Labels
design A design change, especially one which needs input from the design team. feature/rewards

Comments

@srirambv
Copy link
Collaborator

Did you search for similar issues before submitting this one?
Yes

Describe the issue you encountered:
Close button is not aligned with text in payment history window

Expected behavior:
Should be aligned with the text

  • Platform (Win7, 8, 10? macOS? Linux distro?):
    Windows 10 x64

  • Brave Version:
    0.12.8 (possibly on older versions too)

  • Steps to reproduce:

    1. Click on view payment history in Payments
    2. In the view payment history modal, close button is not aligned with the text
  • Screenshot if needed:
    image

  • Any related issues:
    cc: @bradleyrichter @jkup

@srirambv srirambv added design A design change, especially one which needs input from the design team. feature/rewards priority/low labels Nov 17, 2016
@jkup jkup self-assigned this Nov 17, 2016
@jkup jkup added this to the 0.12.11 milestone Nov 17, 2016
@jkup
Copy link
Contributor

jkup commented Nov 17, 2016

Taking this and putting it in 0.12.11 unless anyone else wants it / feels it's a higher priority!

@srirambv
Copy link
Collaborator Author

Its the opposite in Advance settings modal
image

@bradleyrichter
Copy link
Contributor

the X is correct but this needs other cleanup like:

  • title bar has strange spacing in general
  • the button text needs more margin
  • orange and white button text is different
  • no margin below the popup titles
  • tiny arrows in popup
  • popups are missing correct styling
  • bottom buttons should be right aligned

@willy-b
Copy link
Contributor

willy-b commented Nov 18, 2016

I'm happy to take this one

@jkup
Copy link
Contributor

jkup commented Nov 18, 2016

@willy-b Awesome! @bradleyrichter can you upload a spec of what it should look like? Since there are a lot of changes needed let's edit the issue to reflect that!

@jkup
Copy link
Contributor

jkup commented Nov 21, 2016

Removing my assignment for now. @bradleyrichter when you get a spec of the desired design together maybe you can work with @willy-b to get it through?

I'm more than happy to help out / review when the time comes!

@luixxiul
Copy link
Contributor

I must say the code of the Payments modal dialogs in preferences.less and modalOverlay.less is so unorganized that nobody could understand the structure of it clearly. I'm rewriting them to fix not only this, but also #5785 and #5799.

@willy-b
Copy link
Contributor

willy-b commented Nov 23, 2016

@luixxiul as an outsider, I took the minimum change approach when adding the Payment History modal, and probably made things that much messier :-).
Looking forward to seeing / learning from your rewrite of all the Payments/Preferences modal CSS.

@bradleyrichter
Copy link
Contributor

These weren't too far from the intended design. They just needed more polish:

image

image

@luixxiul
Copy link
Contributor

I agree. It is a matter of the code structure.

@bsclifton
Copy link
Member

bsclifton commented Nov 23, 2016

We're going to want to be very careful with changes to CSS. I am generally very supportive, but have I have two concerns:

  • We'll want to search the code / manually test to ensure other pages won't be impacted. If confidence is high with making changes, that is great 😄

  • Many of the webdriver tests use the class names for selectors. We'll want to make sure to try the tests after any changes

With those in concerns in mind, solving this would be great. Thanks for looking into this- your attention to detail is very much appreciated! 😄

@luixxiul
Copy link
Contributor

luixxiul commented Nov 23, 2016

Done, for now :-) Your feedbacks are appreciated!

https://github.com/luixxiul/browser-laptop/commit/2327e9eca53eb4a637dd4c3e3a2a1d1f613c2fbc

@luixxiul
Copy link
Contributor

Closing this in favor of #6202

@luixxiul luixxiul removed this from the 1.0.0 milestone Dec 14, 2016
@luixxiul luixxiul removed their assignment Dec 14, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
design A design change, especially one which needs input from the design team. feature/rewards
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants