-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Fix styling for PR merge section when no checks #11609
Conversation
Unsure which style we want to keep. I opted to use the style from checks variant as it seems the styling applied to no-check variant was accidental due to |
Very minor but is is possible to have the inner border stretch the whole width of the box? |
The line above merge button? |
Yes that one. It should not have margins on the side. |
web_src/less/_repository.less
Outdated
} | ||
|
||
&.no-header { | ||
#avatar-arrow; |
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.
What does this do? Copy the declarations from the other selector? Maybe we don't wanna use too fancy syntax like that because it could be confusing 😉.
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.
It's detached ruleset, I don't know why it was used initially but is used everywhere across this file, presumably to remove code duplication. I just duplicated how it's used everywhere else.
It imports ;:before and ::after rules for showing arrow.
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.
I see, the fact that these use #
make them look like selectors. We should change them (in another PR) to use the @
prefix so it's clearer, like the less docs do:
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.
Agreed
I think I preferred the additional padding around the key. I don't understand why it disappeared and I think it looks crammed without it |
@zeripath It was present only when there were no checks on accident, due to |
And it wasn't really around key, it was around every element, the example doesn't show it but if there was additional line it would had padding too. I can bring the padding back explicitly, however it might make the box quite large when there are 3 or 4 messages. |
Maybe just add padding between siblings? E.g. |
Updated screenshots in first post for future reference. |
make lg-tm work |
please send backport |
Makes styling consistent between two cases. Also removed unnecessary double border.
* Fix inconsistent font size for markdown preview on new PR view (#11565) We use same method for new issue form and issue view, but was missing from new PR view making it one place where markdown preview is inconsistent in font size. (cherry picked from commit 04afd9d) * Fix margin on PR form (#11566) (cherry picked from commit f2a0be1) * Fix margin for attached top header on code review (#11571) Introduced naively by #11463 The margin was being applied too widely. (cherry picked from commit e682a92) * Fix styling for PR merge section when no checks (#11609) Makes styling consistent between two cases. Also removed unnecessary double border. * Normalize avatar radius
Makes styling consistent between two cases. Also removed unnecessary double border.
Makes styling consistent between two cases. Also removed unnecessary double border.
Before:
After: