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

Made the toggling of sticky class relative to unit after discovering … #58

Merged
merged 1 commit into from
Aug 31, 2023

Conversation

darkodevubc
Copy link
Contributor

…that Views pagination can pin the sticknav to the top under certain conditions.

Steps to reproduce the sticky misbehaviour I was seeing:

Fresh D9
Composer require Galactus
Install CLF child manually and enable sticky nav
Generate some content
Create a Views page (paginated, Ajax-enabled, an exposed filter of any kind, enough content in the Global header that you need to scroll down to the filter)
Visit the Views page
Scroll down to filter (nav is sticky)
Click Apply on the filter
Scroll to top of page (nav is still in sticky state)

As admin, the result is that the sticky nav is under the admin_toolbar so it looks like it’s gone. As anon, it’s stuck at the top.

Same behaviour when you scroll down and click the pager.

…that Views pagination can pin the sticknav to the top
@joelpittet
Copy link
Collaborator

@darkodevubc Sorry for the delay, does the patch fix the problem? Introduce any edge cases? I assume you have this working in production with no complaints.

@darkodevubc
Copy link
Contributor Author

I just tested this again locally and it appears that the symptom still exists in Galactus and that this patch fixes it locally. I am using this in production on postdocs.ubc.ca with no other ill effects that I have noticed.

Before screenshot (sticky gets stuck at top of window after ajax filter or pagination):

Screenshot 2023-08-29 at 12 18 01 PM

After screenshot (sticky is not stuck at the top of window after ajax filter or pagination):

Screenshot 2023-08-29 at 12 19 49 PM

@darkodevubc
Copy link
Contributor Author

Would be great if either Joel or James could run through the steps to see if you can reproduce the symptom and whether the patch fixes it for you.

@joelpittet
Copy link
Collaborator

I'll do that, hopefully today

@joelpittet
Copy link
Collaborator

Thanks Darko for retesting it

@occupant
Copy link
Member

@darkodevubc I'll give this a runthrough and see - sorry about the delay!

@occupant occupant self-assigned this Aug 30, 2023
@occupant
Copy link
Member

I was able to reproduce this and the supplied patch corrected the behaviour - only took.... almost exactly 2 years! Lol. Anyway, this looks good to me.

Thanks @darkodevubc ( and @joelpittet for prodding me :) )

@occupant occupant self-requested a review August 31, 2023 00:00
Copy link
Member

@occupant occupant left a comment

Choose a reason for hiding this comment

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

Tested and issue and fix both replicated

@occupant occupant merged commit b932477 into master Aug 31, 2023
@joelpittet joelpittet deleted the stickynav branch August 31, 2023 00:55
@joelpittet
Copy link
Collaborator

Thanks, saved me from setting it up to reproduce the steps @occupant !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants