Redesign concept pages #1346
Redesign concept pages #1346
Conversation
Fix spacing issues, incorrect count for subjects, and All Filters checkbox when no fields exist
Note that this only works on live! `localLink` is false on dev as the content links to the live site.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## redesign-2024 #1346 +/- ##
=================================================
- Coverage 36.35% 36.18% -0.17%
=================================================
Files 476 476
Lines 21186 21321 +135
Branches 7019 6341 -678
=================================================
+ Hits 7702 7716 +14
- Misses 12826 13566 +740
+ Partials 658 39 -619 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -1,5 +1,5 @@ | |||
| import React, {useEffect, useRef, useState} from "react"; | |||
| import {isMobile, isPhy, isTutorOrAbove, PATHS, siteSpecific, useOutsideCallback} from "../../services"; | |||
| import {isAda, isMobile, isPhy, isTutorOrAbove, PATHS, siteSpecific, useOutsideCallback} from "../../services"; | |||
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
To fix the problem, we need to remove the unused import isAda from the import statement on line 2. This will clean up the code and potentially improve performance by reducing the bundle size.
- In general terms, the problem can be fixed by identifying and removing any unused imports.
- Specifically, we will remove
isAdafrom the import statement on line 2 in the filesrc/app/components/elements/ShareLink.tsx. - No additional methods, imports, or definitions are needed to implement this change.
| @@ -1,3 +1,3 @@ | ||
| import React, {useEffect, useRef, useState} from "react"; | ||
| import {isAda, isMobile, isPhy, isTutorOrAbove, PATHS, siteSpecific, useOutsideCallback} from "../../services"; | ||
| import {isMobile, isPhy, isTutorOrAbove, PATHS, siteSpecific, useOutsideCallback} from "../../services"; | ||
| import {selectors, useAppSelector} from "../../state"; |
|
This generic concepts section of SidebarLayout (I would comment on it directly, but I cant here since it was merged as part of the Previous Page Context PR) has slightly different behaviour to the HierarchyFilter in the Question Finder sidebar. In particular when a subject checkbox is a parent of a selected checkbox (and therefore isPartial) here and is clicked, the parent remains selected while the children become deselected. On the QF, the parent AND children become deselected. I prefer this latter behaviour, but either way it should at least be consistent. |
|
Are we abandoning the other parts of the concept sidebar from the designs? Or perhaps handling it later (in a separate card from the listings page)? Waiting for something from content? i.e. the following:
|
We cannot do either of these until the concept pages have been tagged with an audience. At the moment, we cannot infer what context the concept page is if we land on it directly, and even if we come through e.g. GCSE Physics, the hook that uses the previous context doesn't work without the page being landed on being tagged (needed to check whether the page is still applicable to GCSE Physics). There was also a discussion on how the page context interacts with the user's context, which somewhat muddies the question of what stage the "All X Concepts" should display. The result of that was that page context should always take priority, fwiw.
Related questions for concept pages are not currently being augmented in the API. This'll need to be a separate change, and shouldn't touch any of this code, so perhaps we should make a separate card for this. |
Ah, good spot. When you select a child, the parent tag is removed as the search here is an OR, so everything in the entire subject would otherwise be searched. So when you click the parent, it's currently in an off state, so switches on. Will fix. |
Fair enough - I figured it'd be something along these lines. I'd just wanted to know what was going on specifically here wrt these changes. (Also to note it here: This is good - just waiting for release to merge) |
Applies design changes to concept pages.
Note that some changes to concept pages have already been merged owing to the context overhaul PR being based off the first half of this commit. The changes here are an extension of these, cleaning up odd bits on the page.
Note that the
a-linklightbulb icons do not all work in dev, as links to content pages are not seen as local if they starthttps://isaacphysics.org/, as some do.cp_ang_eq_of_motionhas correct links if you need a page to test.