json-ld / json-ld.org

JSON for Linked Data's documentation and playground site
https://json-ld.org/
Other
852 stars 151 forks source link

Redesign homepage by upgrading to Bootstrap 5 #810

Open veyndan opened 11 months ago

veyndan commented 11 months ago

json-ld.org is using Bootstrap 2.x, which is about 10 years old as of now. In an attempt to give the website a fresh lick of paint, I upgraded the ~homepage~ website from Bootstrap 2.x to 5.3.1.

~As I only ugpraded the Bootstrap version for the homepage in this PR, navigating between sub pages will look a bit odd, as the other pages are still using Bootstrap 2.x. I wanted to create this PR so I can get opinions on whether such a change would be accepted, before I go and upgrade the whole site to 5.x!~

~PS: One notable difference is that the icons have been removed. Glyphicons are no longer bundled within Bootstrap, and from what I can tell, getting them from Glyphicons directly costs money. If we wanted to re-add the icons, we would need to get it from another source (e.g., Font Awesome or Bootstrap Icons). I tried using Bootstrap Icons, but I found the icons to look out of place in the redesign, so decided to just rip them out.~

Before After
image image
image image
image image
image image
gkellogg commented 11 months ago

Personally, I think we've accrued too much technical debt on this site, so I appreciate your efforts to move things forward. I think the site works well without the icons, but if we decide on this direction, it would be worth looking at either Font Awesome, or to see about using native Bootstrap icons.

davidlehn commented 11 months ago

Thank you for an update. The site is indeed ancient, but has held up well!

veyndan commented 11 months ago

@davidlehn Thanks for the extra context!

  • A major static site update would move the CSS updates to a single global theme. I recommend waiting to do that rather than updating all the pages by hand. Other than the playground, the site is not complex and I assume modern content design and theme systems of most site generators would work fine.

I'm a bit confused by this point, as there are no CSS updates in this PR, except for obviously referencing CSS that is within Bootstrap.

  • The updated bootstrap looks fine, but I do like icons. I'd use fontawesome. And I think the playground icon is critical. :-)

Happy to put the icons back in, though I still think it looks a bit odd. Will update the PR with them and let you decide.

All in all, what are the immediate next steps with this? Does it make sense for me to continue updating the rest of the website to Bootstrap 5? The homepage at least wasn't too difficult to update, so I'm happy to have a crack at updating the other pages, even if it is just an interim solution until/if we upgrade to some theme system from a site generator.

davidlehn commented 11 months ago

By CSS I meant the general theme of the site. Main layout HTML, CSS, menus, etc, should be in one theme in one place. There should be no need to update pages by hand. This site predates mostly all modern static site generators and we've never tackled the update. I'm suggesting not spending the time with manual updates since that will all get dropped when a better system is adopted. The updates done here could be a base for that updated theme. It should only need to be done once and every page will inherit it.

veyndan commented 11 months ago

@davidlehn Thanks for clearing things up. Judging by #461, that effort has taken/is taking a long time, and I don't particularly want to embark on rehauling the whole site architecture (at least at the moment). Upgrading to Boostrap 5 isn't much effort, as its mostly a find-and-replace job of updating class names.

My original motivations for this PR are to:

  1. Reduce how much we're sending over the wire. Many files depend on both a CDN version of Bootstrap AND a local version of Bootstrap. We should only need one of them. Instead of fighting some old version of Bootstrap to remove the local version, it's easier to just upgrade Bootstrap in its entirety. Bootstrap 5 also removes its dependency on JQuery.
  2. Make it easier to read the website. Currently, I find it fairly difficult to read the website, and with the updated Bootstrap (and some styling changes), I'm finding it a lot easier.

What you're mentioning seems to also make it easier to contribute to the website. I definitely think this is important, but can be done in a separate PR via the usage of some static site generator.

If it's just a concern of increased effort from my part, then I'll happily continue working on this, because I'd rather sink some time into upgrading to Bootstrap 5 and having it merged, than migrating over to 11ty or Jekyll and have the PR sit open for a long time.

veyndan commented 11 months ago

@gkellogg @davidlehn I've upgraded the other pages to use Bootstrap 5. Please see the updated screenshot table!

  1. I've re-included the icons for the section headers on the homepage. Please lmk if you prefer the design with them or without (I'm still on the side of without).
  2. The playground was a bit fiddly to upgrade, so please check it out and make sure it's working as expected. I already did a bit of manual testing on it, but both of you probably have a lot more experience with the tool than I do 😄
  3. Let me know if there are any other sub pages that should be upgraded and that I missed. The pages with a Bootstrap dependency in which I didn't upgrade are: a. playground/dev/index.html b. primer/index.php c. requirements/index.php d. spec/index.php e. test-suite/index.html f. test-suite/vocab.html g. test-suite/vocab_template.haml
gkellogg commented 11 months ago

I can live without the icons, and it actually gives the site a bit more modern look to not have them.

The playground is also a bit fiddly, and really needs its own overhaul (see various issues). I don't think those other pages need to be updated.

veyndan commented 11 months ago

I can live without the icons, and it actually gives the site a bit more modern look to not have them.

Awesome! I just removed them in the latest commit.

Thanks for the detailed list of follow ups. Deleting code is always fun, so I'll happily pick that up after this PR is merged.

dev/index.php can be accessed directly, but I don't think it is directly accessable through site navitagation. That said, it serves a purpose and should eventually be updated.

I went to https://json-ld.org/dev but am getting a 404. Are you referring to a different dev/index.php?

gkellogg commented 11 months ago

That should be spec/index.php. https://json-ld.org/spec/

veyndan commented 11 months ago

@gkellogg Just updated spec/index.php to Bootstrap 5 in the latest commit.

BigBlueHat commented 11 months ago

As the author of #461 (which got bogged down in fiddly complexity...), I'm in favor of getting this merged to move forward (as I don't want this PR to end up in the same state that my hard work did).

We can revisit the move to 11ty in #461 (and deal with the list of "blockers" in this comment) over there or in separate issues/PRs.

Mainly, we just need this site to move forward for the good of the community and the Web, so thank you @veyndan this PR! Let's merge it!

BigBlueHat commented 10 months ago

@davidlehn can we discuss this one soon? I don't want it to end up in the same fate as #461.

gkellogg commented 10 months ago

I think we just need to merge and move on. It makes a definite improvement. @davidlehn?

davidlehn commented 10 months ago

I'm ok with the bootstrap update in theory. However I'm not as enthusiastic about the arbitrary style changes. And there are quite a few problems. Did others test this out or only look at the images? I fired this up locally to compare side by side.

@veyndan: Can you please fix the conflicts. Some updates I did caused them, my bad.

@gkellogg:

I can live without the icons, and it actually gives the site a bit more modern look to not have them.

"modern" is a hard word to interpret in any context. I'd prefer something timeless myself. :-) Not sure why icons are not as popular anymore. Maybe mobile spacing? Maybe hard to find appropriate icons? I did prefer them, and think they are useful for quick visual recognition, and are a hint for those that don't speak our un-internationalized labels. I guess I won't fight everyone on this. Note the screenshot images don't reflect the removed icons.

davidlehn commented 10 months ago