Issue #16243: Remove extra spacing between top image and gray bar in header#16270
Conversation
|
github, generate site |
|
Please make it long, whole width, as before. Difference in rendering should be only no white space between image and that panel |
Sure |
| #banner { | ||
| border-bottom: 1px solid #fff; | ||
| background: url("../images/header-background.png") repeat-x scroll 0 0 transparent; | ||
| height: 97px; |
There was a problem hiding this comment.
Can you tell me how you decided to keep 97px?
IMO you should not give predefined height to HTML elements, there is a chance that it might not work in some devices and it'll give you responsive design issues.
|
There is extra spacing between banner and breadcrumbs bar because of extra margins given to CS logo and red line.
If you remove these margins the extra spacing also gets removed. To solve such CSS issues, you can inspect element and see where the issue is coming from, it will help you to fix them more quickly. |
|
@romani FYI, we had this extra spacing in our old site https://checkstyle.sourceforge.io/version/10.21.1/ Maybe instead of completely removing it, we should decrease its amount. |
|
@SheikhZaeem try to use following type of PR description: #16213 and after sending PR, leave a comment on issue, similar to: #16077 (comment) it helps maintainer to know which task you're targeting. |
|
@Zopsss Thanks a lot again. I will work on them asap. But can you please again explain the part regarding the PR description. |
@Zopsss Well I cannot remove a margin directly from .h1 as it affects all the elements related to that class. Let me figure out something else. 
|
5515ac3 to
812b227
Compare
812b227 to
a85e36a
Compare
|
github, generate site |
|
@romani Well I fixed the problem by removing the h1 margin in the left banner without removing the margin from other places that uses h1 class. Now there is no space between the top image and the content below. However I noticed an additional problem: when we click on the checkstyle image on the top left, it shows something like this: 
But this problem was before my changes. If you want I can fix this also. Thank you |
It is side effect of our AWS temp site. It will work as required in actual release website |
|
I'm taking back my this statement: #16270 (comment)
Old site had this because of some other reasons. It's good that we have fixed it now. |
Check the description of PR I referenced. In there, I have mentioned the task that PR covers. It mentioning the task in description saves some time of maintainers. |
Zopsss
left a comment
There was a problem hiding this comment.
Sorry for the late review, I wasn't active for few days. The CSS LGTM, thanks a lot for the help!
This pull request solves the issue #16243 by removing the unnecessary space between top image and the gray bar. It was done by setting the .banner class height to 97px to ensure proper alignment.
Issue Link:
Fixes: #16243