-
-
Notifications
You must be signed in to change notification settings - Fork 229
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
🎉 automatic breadcrumbs from the tag graph #4343
base: master
Are you sure you want to change the base?
Conversation
Quick links (staging server):
Login:
SVG tester:Number of differences (default views): 0 ✅ Edited: 2025-01-09 23:29:36 UTC |
0342ee6
to
14aac77
Compare
Nice work! A few notes from playing with it: I think it would be great if there were an easier way in the admin to understand if breadcrumbs were set automatically or manually. The final leaf node not being shown is understandable but quite an inconsistency. It might work well to use the slug, split by dash and sentence cased? E.g. "us-airline-travel" -> "Us airline travel". This will clearly sometimes not be perfect but this is what manual overrides are for. What do you think? My hunch is also that we should advertise this staging server quite prominently after the christmas break - I'm pretty sure that Joe, Ed and maybe Max will want to look at quite a few examples to get a feeling for the logic. I think it works very well (e.g. the tie breaking etc) but I anticipate some discussion there. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The types in getParentTagArraysByChildName should be a bit more explicit IMHO, otherwise this looks good!
As I wrote above, I think the semantics of this PR will warrant a bit of scrutiy and discussion and this might mean a few more code changes - if so, ping me again quickly before this gets merged.
Going to chat with Marwa/Ed to talk about what to do with leaf nodes |
Hey @danyx23 🙂 Updates:
breadcrumbs.overrides.demo.mp4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The automatic breadcrumbs work fine AFAICS but the manual ones have two weirdnesses about them:
- When you edit them, you don't immediately see any changes. At least when clicking on "done" I would like the preview to be refreshed. I also don't understand why we have both "done" and "save draft" at this point. I think we should fold the behaviour of "save draft" into the "done" button. Happy to chat about the details of this.
- The other one is that it seems impossible to recreate the behaviour of the current automatic breadcrumbs with manual breadcrumbs. I can leave the last field empty but it still renders a ">" to an empty string in the breadcrumbs in that case. I'd also like to have better instructions for how to fill this in when the manual breadcrumbs is active (something like "Keep the last item empty unless you are editing an SDG page. See the writing guildelines for recommendations for how to pick topic parents")
Once these are tackled this is good to merge!
Resolves #3406
Changes:
ParentTagArray
which is an array of tags. One child tag can have multipleParentTagArray
s e.g. "Nuclear Energy" can be a child of "Energy" and in a different part of the graph, a child of "CO2 Emissions"Rules for the programmatic breadcrumbs:
I considered a "highest average weight" strategy as well, but seeing as weighting is only really used to order nodes relative to one another at each level, I didn't think it would lead to intuitive results.
This means that the ordering of the areas in the tag graph will determine which breadcrumbs trump which. I think this makes the most sense because presumably whichever topic we put at the top of the nav, we think is the most important and therefore its children are more important also, in some abstract way.
Automatic breadcrumbs render slightly differently to manually set breadcrumbs:
Manually set breadcrumbs were made for SDGs at first, where we wanted the leaf node to unclickable. For non-SDG articles, authors have (sensibly) shortened the title for this leaf node, but that's not something we're going to be able to do programmatically, so instead we'll only show topic breadcrumbs, and not a leaf node for the article.