Book Navigation Headings #1485
Book Navigation Headings #1485
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## redesign-2024 #1485 +/- ##
=================================================
- Coverage 40.45% 40.45% -0.01%
=================================================
Files 495 495
Lines 22141 22145 +4
Branches 7342 7344 +2
=================================================
+ Hits 8958 8959 +1
- Misses 12558 13149 +591
+ Partials 625 37 -588 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| <h3 className="mb-3"><Markup encoding="latex">{page.title}</Markup></h3> | ||
| <div className="content-metadata-container d-flex flex-column gap-2"> | ||
| {!!page.gameboards?.length && <BookSectionLink id="questions" title="Questions" />} | ||
| {!!page.relatedContent?.length && <BookSectionLink id="review" title="Review" />} |
There was a problem hiding this comment.
Currently the Review heading covers both page.relatedContent and page.children (and is always present on the page).
If it's meant to be there regardless, so should this link. If its only meant to head relatedContent, that heading should be moved to inside the conditional section. I haven't been part of discussions so I'm not entirely sure what's wanted, but it's not consistent right now either way. (e.g. There's no "Review" button on physics_skills_19/a9)
There was a problem hiding this comment.
Hmm yeah. Tbh, I'm not 100% myself on whether pages will always contain Review content – there's no requirement for them to in the content, they could just set IDs in gameboards and hit publish. We could make it show dependent on both the existence of related content and children.length, but children being empty isn't the only way no content can show – if all children are empty, nothing will appear...
I think it is okay to assume there will be something in the Review block; content will at least check the page and see the "Review" text, which should imply we're expecting it to be there.
There was a problem hiding this comment.
Yeah, I suppose that probably makes the most sense.
It does also now feel strange when "Review" is the only heading - maybe then it shouldn't be navigable to since it's already there on-page? Or perhaps keeping the menu around for consistency is better? I'm not sure - that's probably something worth checking with content (partly because I'm not sure how common it would be to have book pages without question decks once the books are all fully ported to this page type).
Also I didn't bring it up previously because it would get fixed by forcing Review to always be in the menu, but the div probably shouldn't be visible if it has no contents - now impossible, but it had been sticking around as an empty grey rectangle.
Regardless, functionality now seems all good so I'll merge this in and it'll be easier to discuss from there 🤷♀️
There was a problem hiding this comment.
It does also now feel strange when "Review" is the only heading
I do agree, but you can align a section under a "Review" heading with any other page's Review section (even if that one had a section above it) more easily, so for consistency I think we should keep it.
(dependent on https://github.com/isaacphysics/isaac-react-app/tree/redesign/general-improvements-10 – merge this first)
Implement the navigation menu at the top of book pages as seen in the original designs. Bumps Extension Work to the same level of header as Questions and Review to allow this.