localgovdrupal / localgov_base

The base theme for LocalGov Drupal websites.
8 stars 15 forks source link

sticky header settings for localgov_base #569

Closed markconroy closed 1 month ago

markconroy commented 3 months ago

Closes #303


Thanks to Big Blue Door for sponsoring my time to work on this.

msayoung commented 3 months ago

Mark, this is cool.

However I don't think Sticky is working as expected. It seems to behave in the same way as "Scroll"

markconroy commented 3 months ago

Thanks @msayoung

Let's see if I can get that sorted in the next 43 minutes before Merge Tuesday.

markconroy commented 3 months ago

@msayoung it's ready for the next test now.


Thanks to Big Blue Door for sponsoring my time to work on this.

msayoung commented 2 months ago

@markconroy it works nicely now, thanks.

Next issue with the Sticky setting - when tabbing through the content, the focused item should be visible. If you tab backwards through the page it is possible to focus on a thing (a link for example) which is hidden behind the sticky header.

To replicate, visit a page with breadcrumbs are behind the sticky header. TAB backward (Shift + TAB) through the content. Notice that the breadcrumb are not visible when they have focus. Let me know if you need better instructions to replicate.

markconroy commented 2 months ago

Thanks @msayoung - is that something we need to fix? I guess if anyone ever has something sticky on a page and then a scroll a focusable thing behind that sticky item, and then try to focus tab to it, it's going to remain behind the sticky thing.

I can't think of a way around this. Maybe we just note it as a limitation of sticky headers.

msayoung commented 2 months ago

Hey @markconroy

I think its a issue, my description didn't really describe it very well sorry. This article explains it better than I can today: https://tetralogical.com/blog/2023/06/08/focus-in-view/

As described there, it should be fairly easy to fix with a scroll-padding or scroll-margin.

markconroy commented 2 months ago

I understand the issue, and the fix, but for some reason - and maybe this is a good thing - I can't currently replicate it.

I'm trying on Firefox and Chrome and Safari, but every time I tab back to a link (e.g. the breadcrumbs) it all pops in to place and I can see the focussed element properly.

Maybe browsers have fixed this for us?


Thanks to Big Blue Door for sponsoring my time to work on this.

msayoung commented 2 months ago

I can still replicate it, I'm afraid. (Chrome Version 128.0.6613.84 )

On the demo site homepage, scroll untill the three menu blocks (Birth, deaths and marriages etc) are positioned behind the sticky menu. Ensure that your focus is on the More about fostering link

image

Press Shift TAB to tab backwards image

notice the focus is just visible behind the sticky header

Press Shift TAB again, the focus is invisible.


I think I have a fix, I'll push it just now.

markconroy commented 2 months ago

@msayoung are we good to merge this now after your additions?

markconroy commented 1 month ago

@msayoung What's the deal with this one? Will we approve and merge it for Merge Tuesday tomorrow?

msayoung commented 1 month ago

I just want to test it out on a fresh install and an existing theme/subtheme:

fresh install

When I'm running the localgov install profile I get the following error:

  Unable to install theme: 'localgov_base' due to unmet module dependencies: 'localgov_base_helper'.  
markconroy commented 1 month ago

That module is part of this PR so it should be available.

msayoung commented 1 month ago

Test with https://github.com/localgovdrupal/localgov/pull/741

msayoung commented 1 month ago

Runs nicely alongside https://github.com/localgovdrupal/localgov/pull/741