Skip to content
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

Structural fixes #163

Merged
merged 11 commits into from
Oct 11, 2018
Merged

Structural fixes #163

merged 11 commits into from
Oct 11, 2018

Conversation

thomasmassmann
Copy link
Member

This PR fixes the current content column and footer implementation.

The current Footer is build on top of footer portlets. But adding additional portlets to the footer will result in a messy and unstyled footer:

footer before

With some XSLT and a little bit of CSS we are able to create a doormat like footer with up to 4 columns (1 portlet: 12 col width 2 portlets: 6 cols, 3 portlets: 4 cols, 4 portlets: 3 cols and more than 4 portlets: 3 cols each), based on the underlying bootstrap grid:

footer with doormat

The content columns are currently in the "wrong" order in the HTML: column 1 comes before the main content column, which makes it hard to style for mobile views (portlet columns (sidebars) should come at the very end). Bootstrap gives us the option to move columns around, but the order needs to be changed to be able to have all sidebar columns below the main content on mobile width screens.

If both portlet columns are available, they will be shown next to each-other on small screens, and stacked on top of each-other on smallest screens:
3 columns small

3 columns smallest

On desktop wide screens, columns will be shown in the classic 3-column layout:
3 columns desktop

@thomasmassmann
Copy link
Member Author

thomasmassmann commented Oct 2, 2018

Part of #160 .

Copy link
Member

@mauritsvanrees mauritsvanrees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't usually do theming stuff in our company, so I can't really comment on details. But I like the solution that you describe. Makes sense.

@jensens
Copy link
Member

jensens commented Oct 3, 2018

this is planned for 5.2 only right? so we need to branch away for 5.1.

@thomasmassmann
Copy link
Member Author

Those changes are safe for 5.1.x and 5.2.x (even 5.0.x).

@MrTango MrTango self-requested a review October 3, 2018 15:55
Copy link
Contributor

@MrTango MrTango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me in 5.1/5.2

Copy link
Member

@jensens jensens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love that, merge if ready and tests are green!

Copy link
Member

@petschki petschki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great. LGTM!

@thomasmassmann
Copy link
Member Author

@MrTango could you please take a look at the failing 5.2 tests?

@pbauer pbauer merged commit d75853a into master Oct 11, 2018
@jensens jensens deleted the structural-fixes branch February 27, 2020 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants