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

Make the showcase theme selector sticky #514

Merged
merged 2 commits into from
Sep 1, 2023

Conversation

jagthedrummer
Copy link
Contributor

Adding fixed to the theme selector make it stick to the bottom of the browser window instead of being stuck all the way at the bottom of the scroll.

But if we just add the fixed class, then the theme selector will overlap the content at the very bottom of the page and we have no way to get to it.

CleanShot 2023-09-01 at 12 41 45

We can fix that by using a little bit of JS to calculate the height of the theme selector and then set that amount of padding on the bottom of the main container.

Doing that makes it so that there is a scrollbar if the theme selector will overlap any content at the bottom of the main container.

CleanShot 2023-09-01 at 12 38 56

And makes it so that you can scroll all the way down and see all of the content.

CleanShot 2023-09-01 at 12 39 08

Adding `fixed` to the theme selector make it stick to the bottom of the
browser window instead of being stuck all the way at the bottom of the
scroll.

But if we _just_ add the `fixed` class, then the theme selector will
overlap the content at the very bottom of the page and we have no way to
get to it.

We can fix that by using a little bit of JS to calculate the height of
the theme selector and then set that amount of padding on the bottom of
the `main` container.

Doing that makes it so that there is a scrollbar if the theme selector
will overlap any content at the bottom of the `main` container.

And makes it so that you can scroll all the way down and see all of the
content.
@jagthedrummer jagthedrummer requested a review from kaspth September 1, 2023 17:47
@kaspth
Copy link
Contributor

kaspth commented Sep 1, 2023

Ok, something is up with my setup then, because I couldn't get this to look right locally in Safari. Let me pull this down and try again. Sorry for yanking it!

})
</script>

<template id="bullet_train_theme_selects">
<section class="border-t w-full p-4 flex justify-end left-0 bottom-0">
<section class="border-t w-full p-4 flex justify-end left-0 bottom-0 fixed bg-white dark:sc-bg-neutral-900" id="bt-theme-selector">
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, it didn't look to me like we needed bg-white dark:sc-bg-neutral-900 because we got bg-inherit automatically from being in main. Also sc-bg-neutral-900 isn't safe since that comes from Showcase's CSS build and would go away if Showcase stopped using it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without those that div is transparent and content shows through it.

CleanShot 2023-09-01 at 12 58 01

Copy link
Contributor

Choose a reason for hiding this comment

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

Dang, wut. It seems like we don't really ship dark-mode colors, I tried dark:bg-black but that's kinda too much. I don't know if there's a way to transform colors in Tailwind, it would be nice if we didn't have to muck with the build or pollute application.css.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't even get dark:bg-black to do anything. It seemed to fallback to bg-white, which is no good with white text.

@kaspth
Copy link
Contributor

kaspth commented Sep 1, 2023

I also just found out that the container disappears when running with the application.js that uses Turbo when navigating a page.

@jagthedrummer
Copy link
Contributor Author

Also, I just noticed that the theme selector disappears if you click on any of the sidebar links. This happens both in this branch and in main. I suspect it's a turbo thing.

@jagthedrummer
Copy link
Contributor Author

lol, jinx!

@kaspth
Copy link
Contributor

kaspth commented Sep 1, 2023

lol, jinx!

sweet 😂, let me pull your fix down!

Copy link
Contributor

@kaspth kaspth left a comment

Choose a reason for hiding this comment

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

I fixed my local setup and I see this working now! It's just that dark background-color that I don't know what to do with, feel free to merge if that's easier.

})
</script>

<template id="bullet_train_theme_selects">
<section class="border-t w-full p-4 flex justify-end left-0 bottom-0">
<section class="border-t w-full p-4 flex justify-end left-0 bottom-0 fixed bg-white dark:sc-bg-neutral-900" id="bt-theme-selector">
Copy link
Contributor

Choose a reason for hiding this comment

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

Dang, wut. It seems like we don't really ship dark-mode colors, I tried dark:bg-black but that's kinda too much. I don't know if there's a way to transform colors in Tailwind, it would be nice if we didn't have to muck with the build or pollute application.css.

@jagthedrummer jagthedrummer merged commit da01aac into main Sep 1, 2023
1 check passed
@jagthedrummer jagthedrummer deleted the jeremy/showcase-sticky-footer branch September 1, 2023 18:22
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.

2 participants