-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Conversation
Open to @larz0 to take a look |
@SAplayer @JeffryBooher looks good! We should do the same to About dialog as well. (Cc'ing @njx just in case.) |
It still looks weird that the scrollbars are not against the border of the dialog. It will look best if the entire right column scrolled. We can do this by moving the overflow and max-height to |
@larz0 @TomMalbran Pushed. The about-dialog has a changed style as well. Notice that the I didn't merge master in 'cause the new update info url, |
@SAplayer awesome. Just need to make sure the top of "Brackets" needs to line up with the top of the logo. |
|
@@ -964,14 +966,15 @@ a[href^="http"] { | |||
} | |||
.about-text { | |||
// Icon is 120px, so we need at least that much left padding/margin to avoid overlap | |||
margin: 0 10px 0 123px; | |||
margin-left: 123px; |
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.
Let's make it 130px so there's more spacing between the logo and text.
@SAplayer It looks a lot better now. I think that it is ok that the Brackets title scrolls. Maybe that Brackets title should go to the dialog title, so it should say About Brackets. |
@TomMalbran @SAplayer yep I think it's okay to keep "Brackets" title next to the icon. |
Nice. It looks a lot better with the scrollbar at the border of the dialog. |
UX review done. |
Looks good...Merging |
@larz0 This makes the UpdateNotification dialog look less weird.
padding-bottom
so that the scroll content fills the complete.modal-body
max-height
so that more changes are shown at once