Fix flash of incorrect theme upon page load

JoeGandy / ShareX-Custom-Upload

A little PHP script created for uploading custom sharex files to your own webserver

MIT License

166 stars 50 forks source link

Fix flash of incorrect theme upon page load #103

Closed davwheat closed 4 years ago

davwheat commented 4 years ago

This fixes multiple issues, with this being the most significant.

I've simplified the dark CSS to just include the required declarations to override the base bootstrap CSS (only colors).

This issue also fixes...

incorrect hrefs on error page (if you loaded /test/lol , the CSS and JS wouldn't load)
adds icons folder to .htaccess (I thought this would fix the icons not being served, but hasn't)
adds all uploaded files to .gitignore (better git workflows as all the uploads don't appear when running git status)

davwheat commented 4 years ago

It was the issue I set out to fix, but then I decided to fix some other stuff too lol @JoeGandy

theaquarium commented 4 years ago

Hi, thanks for fixing this bug! I thought it was just something I'd have to live with. The only thing that might be worth doing is completely removing the bootstrap class, since that was a feature added to change themes. The theme is used is this ForEvolve/bootstrap-dark (which I probably should have credited somewhere), and the theme only uses the bootstrap and bootstrap-dark classes to do the toggleable themes. The repo also provides CSS files that don't need that class, so it might be worth using those instead and loading the main CSS from the official Bootstrap CDN.

davwheat commented 4 years ago

@theaquarium As I've added a data-theme attribute to body, it should just be a case of swapping bootstrap and bootstrap-dark out. (Or swapping bootstrap to body and dark to body[data-theme=dark])

theaquarium commented 4 years ago

Also, I don't want to argue about style since that's just personal preference, but the use of margin-top instead of padding-top and no min-height for the line numbers was intentional. In my version, the border only shows up when there is a line number, like this:

In the new version, the border fills the entire height of the screen, like this:

This is definitely a matter of opinion about which looks better, but there's one thing that I don't like about the new one and that's when a file is longer than the screen height, the last line looks like this:

The small gap is also there in the old version, but I think it looks worse when the border reaches to the top but not the bottom. However, again, I really don't mind this change and I think both versions look fine. I was trying to find an example of a site that does it my way, but it seems like both Hastebin and GitHub have no border line at all, so maybe that's what we should do instead. Do you have any thoughts about this?

davwheat commented 4 years ago

I don't mind, particularly. The way it looked before was fine, in my opinion. I do like if it stretches all the way down, similar to a page margin.