[4.0] Cassiopea: allow header to be sticky if desired #27579

infograf768 · 2020-01-20T09:50:01Z

Pull Request for Issue #27549

Summary of Changes

Adding css and parameter to make the Header sticky if desired.
Default to No.

FI: I had to add the css in index.php as adding it in grid scss did not work.

Testing Instructions

Patch. NPM needed.

Edit Cassiopea template style, Advanced tab.
Set Sticky Header to Yes

Display frontend and scroll.

Expected result

Note

Works here on Chrome, Firefox, Safari, Opera. Did not test other browsers.
I have no pre-conceived idea about adding this parameter or not, but we now have the choice to add this to Cassiopea if maintainers desire.

C-Lodder · 2020-01-20T11:19:40Z

templates/cassiopeia/index.php

+$stickyHeader = $this->params->get('stickyHeader') ? 'sticky-header' : '';
+
+// Stick the header
+if ($stickyHeader === 'sticky-header')


Please move this styling to the SCSS file.
You can then remove -webkit-sticky too.

As I wrote, I tried that in the grid scss by adding .sticky-header as a child of .container-header but it did not work.
Maybe it could be added by itself. Will test again.

also -webkit-sticky is essential for safari.

or maybe you need in another scss? global?

also -webkit-sticky is essential for safari.

The autoprefixer package hooks in when SCSS compiles to CSS and adds in vendor related things (i.e. the -webkit-sticky property) based on the browsers that it's told to support. So you don't need to manually add all the browser related stuff to the SCSS files, the build step takes care of that for you.

C-Lodder · 2020-01-20T11:21:46Z

templates/cassiopeia/index.php

@@ -93,7 +111,7 @@
 	. $hasClass;
 	echo ($this->direction == 'rtl' ? ' rtl' : '');
 ?>">
-	<div class="grid-child container-header full-width">
+	<div class="grid-child container-header full-width <?php echo $stickyHeader; ?>">


Add the class to the <header>, not the page container

it does not work in that case. In fact it breaks the display.
Or maybe I do not understand what you mean?
if in header class="header", I stand with my statement.

I'll look at this properly tomorrow

Moved css, see below.

We 'may' move it to <header when the curve will be gone. See: #27566

Move to <header>?

jwaisner · 2020-01-20T16:31:03Z

Tested on the frontend using both Firefox and Chrome. Sticky function works even in the frontend editor. Default state is "No" as expected.

The only issue is that the switcher for the "Sticky" setting isnt correct for the "no" state. It should be disabled (grey) not enabled (green).

infograf768 · 2020-01-20T17:22:03Z

@jwaisner
Switcher should now be Ok.

infograf768 · 2020-01-20T17:35:10Z

@C-Lodder
Moved the new css to _layout.scss

npm needed.

templates/cassiopeia/templateDetails.xml

infograf768 · 2020-01-20T17:47:59Z

Folks, if #27566 is tested and merged, this PR will be modified.

jwaisner · 2020-01-20T20:11:13Z

I have tested this item ✅ successfully on 37bdd98

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27579.}

infograf768 · 2020-01-21T07:41:31Z

@C-Lodder
tested here after patching with #27566
I tested applying sticky-header to <header after modiying the css by taking off margin: 0 0 20px 0 !important; as margins are now OK in .container-header header
In this case, it does not work with Chrome or Opera.
WE definitely need to apply the new class to div class="grid-child container-header full-width <?php echo $stickyHeader; ?>">
Will modify this PR as soon as #27566 is merged.

brianteeman · 2020-01-21T08:45:48Z

You do not need to have the webkit line in the scss. The build script deals with that automatically as shown in the screenshots below.

But why are we even adding any css at all. This is a bootstrap template and bootstrap has its own utility class for this already called position-sticky

scss

css

infograf768 · 2020-01-21T09:16:15Z

You do not need to have the webkit line in the scss. The build script deals with that automatically as shown in the screenshots below.

OK. It would work indeed, but no use. I will delete that scss. See below.

But why are we even adding any css at all. This is a bootstrap template and bootstrap has its own utility class for this already called position-sticky

I had tested that. position-sticky is missing an essential part: top: 0;
Without it, it does not work.

But after also adding sticky-top in order to have the following code
$stickyHeader = $this->params->get('stickyHeader') ? 'position-sticky sticky-top' : '';

it works fine in all browsers tested.
Modifying this now as using layout scss is useless.

ChristineWk · 2020-01-21T10:06:08Z

I have tested this item ✅ successfully on f5c1f72

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27579.}

infograf768 · 2020-01-21T10:08:41Z

@jwaisner
Please test again.

Quy · 2020-01-21T20:36:25Z

Do you want to add this parameter to joomla.sql?

(0, 'cassiopeia', 'template', 'cassiopeia', '', 0, 1, 1, 0, '', '{"logoFile":"","fluidContainer":"0","sidebarLeftWidth":"3","sidebarRightWidth":"3"}', 0, NULL, 0, 0),

ciar4n · 2020-01-21T20:59:04Z

I haven't tested this but would this not also stick the banner position? Maybe an issue on small devices if anything large in there.

brianteeman · 2020-01-21T21:02:14Z

yes and the search postion

jwaisner · 2020-01-22T01:18:50Z

I have tested this item ✅ successfully on 9b669f6

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27579.}

infograf768 · 2020-01-22T06:42:06Z

Do you want to add this parameter to joomla.sql?

Not sure it is absolutely necessary as the absence of the parameter = No

infograf768 · 2020-01-22T07:25:48Z

I haven't tested this but would this not also stick the banner position?

Yep, it does. That's why we have a parameter to choose sticky or not, default being No which I would recommend in case of large banner usage..
Using a banner image wider than the mobile window size shrinks the image width (not proportionnaly) and is not recommended anyway.
Basically, as for most aspects, it depends on the site owner decisions.

yes and the search postion

Nope. Search is in the toggler as well as the menu position.

infograf768 · 2020-01-22T07:27:37Z

RTC.

It is now the decision of maintainers to get this feature in or not.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27579.}

ciar4n · 2020-01-22T09:34:47Z

That's why we have a parameter to choose sticky or not, default being No which I would recommend in case of large banner usage..

A fair point but personally I would just stick the navigation/logo. Also may be worth wrapping it in a media query so it only applies to desktops.

infograf768 · 2020-01-22T10:07:26Z

A fair point but personally I would just stick the navigation/logo.

The only way I found was to separate totally the banner from the container-header <div>

Also may be worth wrapping it in a media query so it only applies to desktops.

We indeed could do that.
We would have to override bootstrap. In which scss would you do that?

In any case, if the banner is huge, we would have the same issue in mobile view. Is it worth?

ciar4n · 2020-01-22T10:34:24Z

Try this applied to the .container-header class..

@include media-breakpoint-up(sm) {
  position: relative !important;
}

infograf768 · 2020-01-23T08:28:20Z

Try this applied to the .container-header class..

Would rather be down instead of up, i.e.

@include media-breakpoint-down(sm) {
  position: relative !important;
}

and indeed, for smaller screens, the header would not stick.

@wilsonge @HLeithner
I still think personally that anyone wanting the header to be sticky is not going to add an enormous banner and that keeping the header sticky is also quite useful in mobile mode.
What do you think?

wilsonge · 2020-01-25T23:47:41Z

Personally I find sticky headers on mobiles annoying as it reduces the screen size available for the text. So I'd kinda be ok with it tablet and up 🤷‍♂ but it's just a personal opinion - nothing more

infograf768 · 2020-01-26T06:35:37Z

Personally I find sticky headers on mobiles annoying as it reduces the screen size available for the text. So I'd kinda be ok with it tablet and up 🤷‍♂ but it's just a personal opinion - nothing more

OK, will do in order to get rid of that and concentrate on other matters

infograf768 · 2020-01-26T06:52:03Z

Now unstuck in mobile view.
@ChristineWk @jwaisner @ciar4n
Please test again.

Needs npm

rdeutz · 2020-02-03T21:31:12Z

Removed RTC because a new round of testing is requested from the author of the PR

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27579.}

brianteeman · 2020-02-03T21:47:48Z

I have tested this item ✅ successfully on d6ae643

It works as described but I really don't like just adding more and more parameters

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27579.}

jwaisner · 2020-02-03T22:57:23Z

I have tested this item ✅ successfully on d6ae643

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27579.}

Quy · 2020-02-03T22:59:17Z

RTC

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27579.}

[4.0] Cassiopea: allow header to be sticky if desired

a565fae

joomla-cms-bot added Language Change This is for Translators PR-4.0-dev labels Jan 20, 2020

infograf768 mentioned this pull request Jan 20, 2020

[4.0] [frontend-template] Add sticky site name #27549

Closed

C-Lodder reviewed Jan 20, 2020

View reviewed changes

correcting switcher

48dd750

infograf768 force-pushed the 4.0_sticky-header branch from d436acf to 48dd750 Compare January 20, 2020 17:21

moving css to layout.scss

608e396

Quy reviewed Jan 20, 2020

View reviewed changes

templates/cassiopeia/templateDetails.xml Outdated Show resolved Hide resolved

cs

37bdd98

using bootstrap css

dc230d1

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 22, 2020

unstick in mobile view

236c7d3

infograf768 force-pushed the 4.0_sticky-header branch from 5750193 to 236c7d3 Compare January 26, 2020 06:49

infograf768 and others added 4 commits January 26, 2020 07:52

Merge branch '4.0-dev' into 4.0_sticky-header

ee5db3d

Merge branch '4.0-dev' into 4.0_sticky-header

785c855

Merge branch '4.0-dev' into 4.0_sticky-header

f24e30c

Merge branch '4.0-dev' into 4.0_sticky-header

d6ae643

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 3, 2020

Quy added the RTC This Pull Request is Ready To Commit label Feb 3, 2020

rdeutz merged commit 9d7f97a into joomla:4.0-dev Feb 5, 2020

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 5, 2020

infograf768 deleted the 4.0_sticky-header branch February 5, 2020 08:35

infograf768 added this to the Joomla 4.0 milestone Feb 5, 2020

zero-24 mentioned this pull request Feb 5, 2020

[4.0] Cassiopea: allow header to be sticky if desired joomlagerman/joomla#886

Closed

[4.0] Cassiopea: allow header to be sticky if desired #27579

[4.0] Cassiopea: allow header to be sticky if desired #27579

Conversation

infograf768 commented Jan 20, 2020 • edited

Summary of Changes

Testing Instructions

Expected result

Note

C-Lodder Jan 20, 2020 • edited

Choose a reason for hiding this comment

infograf768 Jan 20, 2020

Choose a reason for hiding this comment

infograf768 Jan 20, 2020

Choose a reason for hiding this comment

infograf768 Jan 20, 2020

Choose a reason for hiding this comment

mbabker Jan 20, 2020

Choose a reason for hiding this comment

C-Lodder Jan 20, 2020

Choose a reason for hiding this comment

infograf768 Jan 20, 2020 • edited

Choose a reason for hiding this comment

C-Lodder Jan 20, 2020 • edited

Choose a reason for hiding this comment

infograf768 Jan 20, 2020

Choose a reason for hiding this comment

Quy Jan 26, 2020

Choose a reason for hiding this comment

jwaisner commented Jan 20, 2020

infograf768 commented Jan 20, 2020

infograf768 commented Jan 20, 2020 • edited

infograf768 commented Jan 20, 2020

jwaisner commented Jan 20, 2020

infograf768 commented Jan 21, 2020

brianteeman commented Jan 21, 2020

scss

css

infograf768 commented Jan 21, 2020

ChristineWk commented Jan 21, 2020

infograf768 commented Jan 21, 2020

Quy commented Jan 21, 2020

ciar4n commented Jan 21, 2020

brianteeman commented Jan 21, 2020

jwaisner commented Jan 22, 2020

infograf768 commented Jan 22, 2020 • edited

infograf768 commented Jan 22, 2020

infograf768 commented Jan 22, 2020

ciar4n commented Jan 22, 2020

infograf768 commented Jan 22, 2020 • edited

ciar4n commented Jan 22, 2020

infograf768 commented Jan 23, 2020

wilsonge commented Jan 25, 2020

infograf768 commented Jan 26, 2020

infograf768 commented Jan 26, 2020 • edited

rdeutz commented Feb 3, 2020

brianteeman commented Feb 3, 2020

jwaisner commented Feb 3, 2020

Quy commented Feb 3, 2020

infograf768 commented Jan 20, 2020 •

edited

C-Lodder Jan 20, 2020 •

edited

infograf768 Jan 20, 2020 •

edited

C-Lodder Jan 20, 2020 •

edited

infograf768 commented Jan 20, 2020 •

edited

infograf768 commented Jan 22, 2020 •

edited

infograf768 commented Jan 22, 2020 •

edited

infograf768 commented Jan 26, 2020 •

edited