civicrm / org.civicrm.shoreditch

Other
44 stars 59 forks source link

Strangely styled panels #24

Open colemanw opened 7 years ago

colemanw commented 7 years ago

This line of css makes panels look odd:

#bootstrap-theme .panel-primary:not(.panel-w-subheading)>.panel-heading, #bootstrap-theme .panel-primary.panel-w-subheading>.panel-subheading, #bootstrap-theme .panel-success:not(.panel-w-subheading)>.panel-heading, #bootstrap-theme .panel-success.panel-w-subheading>.panel-subheading, #bootstrap-theme .panel-info:not(.panel-w-subheading)>.panel-heading, #bootstrap-theme .panel-info.panel-w-subheading>.panel-subheading, #bootstrap-theme .panel-warning:not(.panel-w-subheading)>.panel-heading, #bootstrap-theme .panel-warning.panel-w-subheading>.panel-subheading, #bootstrap-theme .panel-danger:not(.panel-w-subheading)>.panel-heading, #bootstrap-theme .panel-danger.panel-w-subheading>.panel-subheading {
    border-bottom-right-radius: 2px;
    border-bottom-left-radius: 2px;
    margin-bottom: 10px;
}

What is the reason for this extra space between the panel header and the panel content?

AkA84 commented 7 years ago

@colemanw not sure what you mean by odd, please add a screenshot to clarify

Anyway the margin-bottom is there because panels with contextual classes (.panel-primary, .panel-success, etc) have floating headings

floating-heading

As opposed to default panels (.panel-default)

panel-default

The same goes with panels with subheadings

panel-subheading
colemanw commented 7 years ago

@AkA84 Yes that is the problem. Panels with contextual classes are forced to have floating headings by the Shoreditch Theme. This is not standard bootstrap behavior.

I recommend you add a new panel-floating-heading class instead of forcing this on all panels.

colemanw commented 7 years ago

Here's my problem. Trying to do a simple layout for the new Api4 Explorer using standard bootstrap styles: explorer

I had to override the strange header styling to get all the panels to align correctly (the blue ones had extra spacing). I don't think I ought to have to do that. Again, if you want floating headers for a particular screen you're creating, that's fine. Just add a new panel-floating-header class or similar. But I don't think it should be forced on all panels everywhere.

AkA84 commented 7 years ago

@colemanw it really depends on how we want the theme to look, i don't think it really matters whether the look is standard bootstrap or not: the component is indeed standard bootstrap, it's just the look of the component that is customized

The theme has been designed (as seen also in the styleguide) to have floating headers for all contextual panels, so that means that you (or whoever designed that page) are currently trying to force something that the theme does not support and wasn't intended to, although i see that if you want to horizontally align a default and a contextual panel, the floating header would prevent a correct alignment.

Of course this must lead to a conversation about what needs to change, the theme or the design of the page (that's why the theme is in early alpha btw!). cc @jamienovick @guanhuan

jamienovick commented 6 years ago

@jonscreat can we close this ticket as we now agree we shouldn't have any floating panel headings yes?

jamienovick commented 6 years ago

Or is there more to do?

ghost commented 6 years ago

@jamienovick This can be closed for me. Floating panel headers shouldn't be a thing anymore.

jamienovick commented 6 years ago

@AkA84 Do we need to do anything to get rid of floating panel headers or can this be closed?

AkA84 commented 6 years ago

Well we need to amend the theme's style, as it currently does have floating panel headers