Skip to content

Book Navigation Headings #1485

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
May 29, 2025
Merged

Book Navigation Headings #1485

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7424279
Rename `icon-arrow-right`; add down arrow
jacbn May 27, 2025
be42d1b
Add book navigation component
jacbn May 27, 2025
d9ac59c
Merge branch 'redesign-2024' into redesign/book-nav-headings
sjd210 May 28, 2025
6b7ab0b
Remove check on `relatedContent` to show Review link
jacbn May 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions src/app/components/elements/BookPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,34 @@ import {IsaacBookDetailPageDTO} from "../../../IsaacApiTypes";
import {TeacherNotes} from "./TeacherNotes";
import {EditContentButton} from "./EditContentButton";

const BookSectionLink = ({id, title}: { id: string; title: string }) => {
return <a className="d-flex align-items-center ms-1 ps-2 invert-underline" href={`#${id}`}>
<i className="icon icon-arrow-down me-2"/>
{title}
</a>;
};

export const BookPage = ({ page }: { page: IsaacBookDetailPageDTO }) => {

return <div className="book-page">
<>
<h3 className="mb-3">
{page.subtitle && <span className="me-2 text-theme">{page.subtitle} </span>}
{page.subtitle && <span className="me-3 text-theme">{page.subtitle} </span>}
{page.title}
</h3>

<EditContentButton doc={page}/>

<TeacherNotes notes={page.teacherNotes} />

<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" />}
Copy link
Contributor

@sjd210 sjd210 May 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@sjd210 sjd210 May 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🤷‍♀️

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

{!!page.extensionGameboards?.length && <BookSectionLink id="extension" title="Extension work" />}
</div>

{!!page.gameboards?.length && <>
<h4 className="mb-3" id="resources">Questions</h4>
<h4 className="mb-3" id="questions">Questions</h4>
<div className="mt-3 mb-5 list-results-container p-2">
<ListView
items={page.gameboards.map(g => ({...g, type: "gameboard"}))}
Expand All @@ -42,7 +55,7 @@ export const BookPage = ({ page }: { page: IsaacBookDetailPageDTO }) => {
</IsaacContentValueOrChildren>

{!!page.extensionGameboards?.length && <>
<h5 className="mt-5 mb-3" id="extension">Extension work</h5>
<h4 className="mt-4 mb-3" id="extension">Extension work</h4>
<span>Expand your boundaries by having a go at these additional extension questions.</span>
<div className="mt-3 mb-5 list-results-container p-2">
<ListView
Expand Down
8 changes: 4 additions & 4 deletions src/app/components/elements/layout/SidebarLayout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ const RelatedContentSidebar = (props: RelatedContentSidebarProps) => {
</ul>
: <>
There are no related concepts for this {pageType}.
{isFullyDefinedContext(pageContext) && <AffixButton color="keyline" className="mt-3 w-100" tag={Link} to={extendUrl(pageContext, "concepts")} affix={{affix: "icon-right", position: "suffix", type: "icon"}}>
{isFullyDefinedContext(pageContext) && <AffixButton color="keyline" className="mt-3 w-100" tag={Link} to={extendUrl(pageContext, "concepts")} affix={{affix: "icon-arrow-right", position: "suffix", type: "icon"}}>
See all concepts for {getHumanContext(pageContext)}
</AffixButton>}
</>
Expand Down Expand Up @@ -226,7 +226,7 @@ const RelatedContentSidebar = (props: RelatedContentSidebarProps) => {
</>
: <>
There are no related questions for this {pageType}.
{isFullyDefinedContext(pageContext) && <AffixButton color="keyline" className="mt-3 w-100" tag={Link} to={extendUrl(pageContext, "questions")} affix={{affix: "icon-right", position: "suffix", type: "icon"}}>
{isFullyDefinedContext(pageContext) && <AffixButton color="keyline" className="mt-3 w-100" tag={Link} to={extendUrl(pageContext, "questions")} affix={{affix: "icon-arrow-right", position: "suffix", type: "icon"}}>
See all questions for {getHumanContext(pageContext)}
</AffixButton>}
</>
Expand Down Expand Up @@ -654,7 +654,7 @@ export const PracticeQuizzesSidebar = (props: PracticeQuizzesSidebarProps) => {
<div className="sidebar-help">
<p>You can see all of the tests that you have in progress or have completed in your My Isaac:</p>
<AffixButton size="md" color="keyline" tag={Link} to="/tests" affix={{
affix: "icon-right",
affix: "icon-arrow-right",
position: "suffix",
type: "icon"
}}>
Expand Down Expand Up @@ -1512,7 +1512,7 @@ export const GenericPageSidebar = (props: ContentSidebarProps) => {
// Default sidebar for general pages that don't have a custom sidebar
return <ContentSidebar buttonTitle="Options" hideButton optionBar={props.optionBar}>
<div className="section-divider"/>
<AffixButton color="keyline" tag={Link} to={"/"} affix={{affix: "icon-right", position: "suffix", type: "icon"}}>
<AffixButton color="keyline" tag={Link} to={"/"} affix={{affix: "icon-arrow-right", position: "suffix", type: "icon"}}>
Go to homepage
</AffixButton>
</ContentSidebar>;
Expand Down
5 changes: 2 additions & 3 deletions src/app/components/elements/list-groups/ListView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,12 @@ export const QuizListViewItem = ({item, isQuizSetter, useViewQuizLink, ...rest}:
const dispatch = useAppDispatch();
const itemSubject = tags.getSpecifiedTag(TAG_LEVEL.subject, item.tags as TAG_ID[])?.id as Subject;
const quizButton = isQuizSetter ?
<AffixButton size="md" color="solid" onClick={() => (dispatch(showQuizSettingModal(item)))} affix={{ affix: "icon-right", position: "suffix", type: "icon" }}>
<AffixButton size="md" color="solid" onClick={() => (dispatch(showQuizSettingModal(item)))} affix={{ affix: "icon-arrow-right", position: "suffix", type: "icon" }}>
Set test
</AffixButton> :
<AffixButton size="md" color="solid" to={`/${documentTypePathPrefix[DOCUMENT_TYPE.QUIZ]}/attempt/${item.id}`} tag={Link} affix={{ affix: "icon-right", position: "suffix", type: "icon" }}>
<AffixButton size="md" color="solid" to={`/${documentTypePathPrefix[DOCUMENT_TYPE.QUIZ]}/attempt/${item.id}`} tag={Link} affix={{ affix: "icon-arrow-right", position: "suffix", type: "icon" }}>
Take the test
</AffixButton>;

return <AbstractListViewItem
icon={{type: "hex", icon: "icon-tests", size: "lg"}}
title={item.title ?? ""}
Expand Down
2 changes: 1 addition & 1 deletion src/app/components/pages/Concepts.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ export const Concepts = withRouter((props: RouteComponentProps) => {
<p className="me-3">The concepts shown on this page have been filtered to only show those that are relevant to {getHumanContext(pageContext)}.</p>
<AffixButton size="md" color="keyline" tag={Link} to="/concepts" className="ms-auto"
affix={{
affix: "icon-right",
affix: "icon-arrow-right",
position: "suffix",
type: "icon"
}}>
Expand Down
2 changes: 1 addition & 1 deletion src/app/components/pages/QuestionFinder.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ export const QuestionFinder = withRouter(({location}: RouteComponentProps) => {
<p className="me-3">The questions shown on this page have been filtered to only show those that are relevant to {getHumanContext(pageContext)}.</p>
<AffixButton size="md" color="keyline" tag={Link} to="/questions" className="ms-auto"
affix={{
affix: "icon-right",
affix: "icon-arrow-right",
position: "suffix",
type: "icon"
}}>
Expand Down
2 changes: 1 addition & 1 deletion src/app/components/pages/quizzes/SetQuizzes.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ function QuizAssignment({assignedGroups, index}: QuizAssignmentProps) {
<span className="manage-quiz-title me-3">{quizTitle}</span>
</Col>
<Col className="d-flex align-items-center justify-content-end col-5 col-sm-4 col-md-6">
<AffixButton size="sm" affix={{ affix: "icon-right", position: "suffix", type: "icon" }} className="me-3"
<AffixButton size="sm" affix={{ affix: "icon-arrow-right", position: "suffix", type: "icon" }} className="me-3"
onClick={(e) => {
// eslint-disable-next-line @typescript-eslint/no-unused-expressions
assignment.quizSummary && dispatch(showQuizSettingModal(assignment.quizSummary));
Expand Down
7 changes: 6 additions & 1 deletion src/scss/phy/icons.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,15 @@
@include svg-icon('/assets/phy/icons/redesign/sidebar.svg', 16px, 16px);
}

.icon-right {
.icon-arrow-right {
@include svg-icon('/assets/phy/icons/redesign/arrow-right.svg', 16px, 16px);
}

.icon-arrow-down {
@include svg-icon('/assets/phy/icons/redesign/arrow-right.svg', 16px, 16px);
transform: rotate(90deg);
}

.icon-chevron-down {
@include svg-icon('/assets/phy/icons/redesign/chevron-down.svg', 16px, 16px);
}
Expand Down
Loading