Skip to content
This repository has been archived by the owner on Apr 5, 2022. It is now read-only.

Allow and configure hiding the intro on mobile pages #108

Merged
merged 3 commits into from
Mar 13, 2020

Conversation

matt9j
Copy link
Contributor

@matt9j matt9j commented Jan 23, 2020

Description

This commit adds css, and template logic to hide the intro on mobile devices (when in linear layout and not grid layout) for pages that are not the homepage.

I added configuration parameters to config.toml to configure if it is always hidden on mobile, never hidden on mobile, or only hidden on pages that are not the homepage. I used two boolean config parameters, which does create a fourth redundant state where hideOnMobile = false and alwaysOnHomepage = true leads to the same outcome as hideOnMobile = false and alwaysOnHomepage = false. This configuration could be changed to some kind of state enum if people think that would be better, at the expense of the logic getting a little more complicated in the template.

Motivation and Context

The intro section can be quite long, especially when a picture is enabled. See the screenshots below for examples. By default the intro appears on the left on computer screen sizes, but above the content on mobile devices. Before this change the intro appeared on every page, even the detailed pages of specific blog posts.

Screenshots:

Current desktop layout of a post (or content list)

The desktop layout has a nice grid structure with the intro and post content visible at the same time. The proposed PR does not change this layout at all.

Grid Layout

Current mobile layout of a post (or content list)

Note the redundant intro that users have likely already seen taking up a large fraction of the screen

linear layout

Proposed mobile layout change

This PR proposes allowing users to hide the intro block on mobile pages if they desire. This lets the post page lead with post content rather than the redundant intro. As currently implemented the intro will be hidden both on post detail pages and "deeper" site list pages like the list of blog posts or the list of categories.
linear layout without intro

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

The intro section can be quite long, especially when a picture is
enabled. By default the intro appears on the left on computer screen
sizes, but above the content on mobile devices. This commit adds
configuration parameters, css, and template logic to hide the intro on
mobile devices, and configure if it is always hidden, or just on
content pages that are not the homepage.
@pacollins
Copy link
Owner

I'm not opposed to this change. I will take a look at it soon and evaluate.

Thanks for the PR!

@matt9j
Copy link
Contributor Author

matt9j commented Feb 22, 2020

No problem, thanks for building and maintaining the theme. Let me know if you have any questions or if you would like me to make any changes.

Copy link
Owner

@pacollins pacollins left a comment

Choose a reason for hiding this comment

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

Everything looks great, but to ensure backwards compatibility, the default state should exclude this feature.

Comment on lines 64 to 67
# You may not want to display a long intro above content on small screens.
hideOnMobile = true
# But you may want to always display the intro on the homepage.
alwaysOnHomepage = true
Copy link
Owner

Choose a reason for hiding this comment

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

Changing these to false should cover the changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

#site-intro.hidden-on-mobile {
display: none;

@include for-desktop-up {
Copy link
Owner

Choose a reason for hiding this comment

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

This should probably be for-laptop-up of the goal is to remove for mobile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I agree, but in agreeing realized that my goal is to remove this for any of the single column layouts, not just on mobile. In main.scss #wrapper the two column layout is defined for-desktop-up. I think the best thing to do is actually rename the parameter to refer to hiding the intro on single columns layouts instead of "on mobile" for clarity.

Copy link
Owner

Choose a reason for hiding this comment

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

I believe this does work to solve the issue.

This commit sets the example site into hiding parameters to False to
better track the exisiting theme behavior for backwards
compatibility. If these parameters are not included in a site config,
the intro will also not be hidden, maintaining existing behavior.
When referring to intro hiding, the intended behavior is to hide the
intro on all single column layouts, not just ones "on mobile." Notably
the `for-laptup-up` media type used the single column layout, but the
name and intention is for "non-mobile" devices. To more clearly
communicate intent this commit renames hideOnMobile to
hideWhenSingleColumn and changes the relevant references from mobile
hiding to single column hiding.
Copy link
Owner

@pacollins pacollins left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I'm sure this is a feature that people will appreciate!

#site-intro.hidden-on-mobile {
display: none;

@include for-desktop-up {
Copy link
Owner

Choose a reason for hiding this comment

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

I believe this does work to solve the issue.

@pacollins pacollins merged commit ca72f1e into pacollins:master Mar 13, 2020
@matt9j matt9j deleted the mobile-intro branch March 13, 2020 22:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants