Closed joelsjlee closed 3 months ago
@joelsjlee I've pushed up my changes! Take a look and make any tweaks. You can make the merge whenever you're ready.
Everything here looks great! One last thing I forgot to mention before I merge, when I'm generating the site and testing it out on a local server, clicking on the explore.html page that Julia set up gives a 404 (and subsequently the link to it in the recommendation box does the same). I'm assuming this is because the paths aren't set up yet in the ant build file (?). Should we add this to this pull request?
That would be my guess--if so, yes, it sounds like adding this to the PR would make sense!
best, Julia
On May 29, 2024, at 12:00 PM, Joel @.***> wrote:
Everything here looks great! One last thing I forgot to mention before I merge, when I'm generating the site and testing it out on a local server, clicking on the explore.html page that Julia set up gives a 404 (and subsequently the link to it in the recommendation box does the same). I'm assuming this is because the paths aren't set up yet in the ant build file (?). Should we add this to this pull request? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>
Ah, I didn't notice the page was in a new directory. Let me fix the Ant build...
Done!
I added a change that basically adds some conditionals to the population of recommendations. If an article has no recommendations in any of the TSV files, then the recommendation box should not populate. If an article has some recommendations (ex: tfidf and SPECTER but not keyword), then the omitted section should not show in the recommendation box.
@joelsjlee This looks great! Are there articles I can test this on, to see the conditionality in action?
Yes! I believe 000672 should have SPECTER recommendations omitted, and 000580 should have all of the recommendations omitted.
I'm waiting on the last up-to-date version of the TSVs that removes some of the purposefully omitted articles from being recommended. Once that is all squared away and the TSVs are passing all of our tests, (hopefully by this week) everything should be all set.
@joelsjlee Just tested the new code and it looks great! There's only one small problem: When the Recommendations box is omitted, the toolbars at the top and bottom of the page still show the "See Recommendations" link. Could you fix that?
I've added a rudimentary fix... I couldn't figure out if it was possible to have some kind of interaction between the toolbar and recommendation templates to get that part removed within the existing conditional in the recommendation box so I ended up copying and pasting some code. Is there a more elegant solution here?
Good question! I just pushed up an attempt, but don't have it working quite yet. I added a global variable to say, true or false, if the current article has any recommendation in any TSV. It works a bit too well at excluding the Recommendations box and links, though — they're not showing up anymore. (I probably messed up the test.)
I think I got it! Needed to put the "m" flag on matches()
so each row counts as a new line-beginning (^
).
This looks great! Thanks so much for the fix Ash. I just updated to a stable version of the TSVs. There was a bit of debugging to do, and removing some articles, but everything should be good now. Shall I go ahead and merge the PR?
Yeah, go for it! 🎉
This PR is to add the recommendations box at the end of article to the DHQ website. It takes in three tsv files which I have added in a subfolder
data/dhq-recs
, and runs xslt code (dhq2html.xsl
) over the files to generate the top 5 recommendations for each of the recommendation systems. I have edited minimally the css to include these recommendations in the same styling as the Abstract and I have implemented the namespace in thedhq2html.xsl
file to use for a function.This PR also merges into it Julia's
add_recommendations
branch.A couple things left to still do:
add_recommendations
branch, and edit the recommendation text inside the box to link out to theexplore.html
page.Also, this pull request is being done from
static_site_generation
intostatic_site_for-merging
because of a mishap from my part, shoutout and big thanks to Ash for coming up with a solution for this.