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

FEATURE: Scroll-based logo on mobile #6632

Merged
merged 21 commits into from Nov 22, 2018
Merged

FEATURE: Scroll-based logo on mobile #6632

merged 21 commits into from Nov 22, 2018

Conversation

hnb-ku
Copy link
Contributor

@hnb-ku hnb-ku commented Nov 20, 2018

This PR adds the ability to show the small logo and topic information in the header on mobile while the user is scrolling down.

Previous related discussions can be found here https://meta.discourse.org/t/smart-mobile-header/99846

Re-ordering the elements in the header-contents widget came up while working on this. So, this PR also re-orders those elements and converts the header to flexbox

There are a few places where I skipped `this.get('foo')` and used `this.foo` because faster is better and it seems to be working without issue when I tested.
The order here was incorrect, the order we want is:

1- logo
2- header topic info
3- user panels
This commit adds a height attribute to the large logo since that eliminates the need for a number of CSS rules to control its dimensions.

The height and width are set for the small logo but only the height is set for the large logo. We set its width with one small CSS rule
some of these are moved to the header stylesheet
some of these are moved to the header stylesheet
This is now moved to the header stylesheet
@hnb-ku hnb-ku changed the title Scroll-based logo on mobile FEATURE: Scroll-based logo on mobile Nov 20, 2018
@hnb-ku hnb-ku removed the request for review from SamSaffron November 20, 2018 08:18
Copy link
Member

@awesomerobot awesomerobot left a comment

Choose a reason for hiding this comment

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

Seems good overall! just a couple issues on small screens

app/assets/stylesheets/common/base/header.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/common/base/header.scss Outdated Show resolved Hide resolved
@discourse discourse deleted a comment from discoursedangerbot Nov 21, 2018
@awesomerobot
Copy link
Member

This is working as expected in Chrome, but tags aren't truncating in Safari... I recall running into this before but don't remember the solution. I'm going to look into this again a bit later and I'll let you know if I can figure it out.

screen shot 2018-11-21 at 6 02 03 pm

@awesomerobot
Copy link
Member

Ah, found it — if you add display:inline; to .discourse-tag it's fixed in Safari. Looks like it's good to merge otherwise!

@awesomerobot awesomerobot merged commit ee6c017 into discourse:master Nov 22, 2018
@hnb-ku hnb-ku deleted the smart-mobile-header branch November 22, 2018 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants