CU-DBMI / set-website

Website for the software engineering team (SET)
https://cu-dbmi.github.io/set-website
BSD 3-Clause "New" or "Revised" License
1 stars 3 forks source link

Update to rely on HTML table rendering instead of images #32

Closed d33bs closed 7 months ago

d33bs commented 8 months ago

This PR addresses a prior issue with HTML rendering which involved static image workarounds (fixed in a recent release of LWT).

Closes #31

github-actions[bot] commented 8 months ago

PR Preview Action v1.4.7 :---: :rocket: Deployed preview to https://CU-DBMI.github.io/set-website/preview/pr-32/ on branch gh-pages at 2024-04-22 21:32 UTC

d33bs commented 8 months ago

Looks like the build fails based on a bundler and Ruby dependency challenge.

7:53:32 AM: ERROR:  Error installing bundler:
7:53:32 AM:     There are no versions of bundler (= 2.5.6) compatible with your Ruby & RubyGems
7:53:32 AM:     bundler requires Ruby version >= 3.0.0. The current ruby version is 2.7.2.137.
vincerubinetti commented 8 months ago

Netlify by default comes with Ruby 2.7 installed, and a recent template version (I think v1.2.0) updated Gem versions such that we need Ruby > 3.0. This is okay because Netlify hasn't been the recommended way to do PR previews since v1.0.0. The PR preview action seems to have worked fine.

I see that the post has some dangling </table> tags that weren't closed properly.

vincerubinetti commented 8 months ago

Playing around locally, it seems like the <strong> before the nested <table> is what's causing the issue. If I wrap the two in a <div>, such that the <td> only has one child, it works. This must be some bug in the markdown parser. At least it's not an issue with the template this time.

vincerubinetti commented 8 months ago

Turns out this is an issue with jekyll spaceship, very weird:

https://github.com/jeffreytse/jekyll-spaceship/issues/97

vincerubinetti commented 7 months ago

Unfortunately, it seems like jekyll-spaceship hasn't been updated in years and that bug is unlikely to be fixed. But it provides too much value to be disabled outright imo. So the <div> wrapper work-around would be recommended here.

I would update it myself, but it's a fork and I don't think I have permissions. If you want to update this, we can merge and close.

d33bs commented 7 months ago

Thanks for the follow up, I tried to wrap the nested tables and saw that the rendering still looks a bit unusual. Maybe it's the multiple nested tables that's throwing it off? I can also close without merge here if it'd be best.

image

vincerubinetti commented 7 months ago

The opening <div> should go above the <strong>. The bug happens when a <td> has more than one child.

d33bs commented 7 months ago

Thank you @vincerubinetti for your help with this! Merging this in.