Add project and scope filters to checks side panel #1428

jolierabideau · 2025-01-24T21:17:02Z

PR for issue PT-1480

Checks side panel web view

Updated checkInputRange to account for the selected scope. If "All" sets the end of the range to Canon.lastBook.
Create a state variable to track the selected project ID, set it to the initial project ID from the editor.
Add the ChecksProjectFilter and ChecksScopeFilter to the top of the web view.

ChecksProjectFilter

Gets the project short and full names for all editable projects, adds them to a dictionary where the key is the project ID.
Uses the FilterPopover with the child element as a RadioGroup with a RadioGroupItem and Label for each project.
Currently disabled (as the issue suggests), but you are able to select different projects and have the results update.

ChecksScopeFilter

Uses the FilterPopover with the child element as a RadioGroup with a RadioGroupItem and Label for each value in the CheckScopes enum.

FilterPopover

Shared Popover component for the filters in the checks side panel, since they have similar button and popover styles.
The child element can be a RadioGroup with items or a group of Checkboxes (for the project type dropdown).
Renders a Popover with Trigger, PopoverContent, a space for the group label and a Separator.

platform-bible-react

Export Popover component, and add comments to popover.tsx for the generated documentation.

This change is

…ecks-project-scope-filter

tjcouch-sil

Looks great! Thanks for the great work on this. Just a few little things to adjust.

Reviewed 15 of 15 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @jolierabideau)

extensions/src/platform-scripture/src/checks-side-panel.web-view.tsx line 45 at r1 (raw file):

  const [scrRef, setScrRef, ,] = useWebViewScrollGroupScrRef();
  const [selectedCheckId, setSelectedCheckId] = useState<string>('');
  const [scope, setScope] = useState<CheckScopes>(CheckScopes.Chapter);

Should this be a useWebViewState so the scope persists across refreshes and sessions and such?

extensions/src/platform-scripture/src/checks-side-panel.web-view.tsx line 233 at r1 (raw file):

  const handleSelectProject = useCallback(
    (newProjectId: string) => {
      setProjectId(newProjectId);

FYI instead of using useState to track changes to the project id, you can instead directly use the projectId from the web view props and use another prop updateWebViewDefinition to update projectId :) then it will work across refreshes and such. Right now, the project id will be set back to whatever it was opened with every time because the web view prop will stay the same. Since it's all disabled, anyway, it doesn't really matter right now

extensions/src/platform-scripture/src/checks/configure-checks/checks-scope-filter.component.tsx line 44 at r1 (raw file):

 */
export default function ChecksScopeFilter({ handleSelectScope }: ChecksScopeFilterProps) {
  const [selectedScope, setSelectedScope] = useState<CheckScopes>(CheckScopes.Chapter);

The selected scope should be passed in as a prop so the first selection is correct if not Chapter

extensions/src/platform-scripture/src/checks/configure-checks/checks-project-filter.component.tsx line 84 at r1 (raw file):

  const writeProjectName = useCallback((fullName: string, shortName: string) => {
    return `${fullName} (${shortName})`;

Concatenating strings should be done with a localized format string like this one for example

extensions/src/platform-scripture/src/checks-side-panel.web-view-provider.ts line 10 at r1 (raw file):

import papi from '@papi/backend';
import checksSidePanelWebView from './checks-side-panel.web-view?inline';
import tailwindStyles from './tailwind.css?inline';

extensions/src/platform-scripture/contributions/localizedStrings.json line 56 at r1 (raw file):

      "%webView_checksSidePanel_focusedCheckDropdown_denyItem%": "Deny",
      "%webView_checksSidePanel_focusedCheckDropdown_settingsItem%": "Open settings and inventories",
      "%webView_checksSidePanel_loadingCheckResults%": "Loading Check Results...",

FYI this is a meaning change, so we would normally want to create a new string instead of changing the existing one. If it was fixing a typo or something, that would be fine. But if someone else might already be using this string or if it has already been translated to another language, changing the meaning of a string is dangerous. As such, we should normally create a new string on changes in meaning. BUT it's not a huge deal this early and with such a specific string that almost confidently hasn't been used elsewhere. It's fine to leave it this time.

Sebastian-ubs

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @jolierabideau)

a discussion (no related file):
Hi Jolie, when trying I found some issues:

The project dropdown is for some reason disabled. This creates an unexpected combination of 3 colors with the bcv and the dropdown to the right. I cannot think of a reason, where we want it to be disabled
The last screenshot looks correct.
I am not able to persist any check configuration, (reported in https://paratextstudio.atlassian.net/browse/PT-2138, this does not only affect this pr)
ideally checks sidepanel should not allow to change the scrollgroup (like in PT9). until this is possible: Changing the scroll group fon the checks sidepanel should also change it for the project. Similarly when changing the scroll group on the project, it should update for the checks sidepanel.
On opening the checks sidepanel I got the following error:

[1] 13:43:36
[1] [extension host error]
[1] An unexpected error occurred while executing "object:platformScripture.checkAggregator-data.setActiveRanges" JSON-RPC method: Error: Die Methode "object:dotNetCheckRunner-data.setActiveRanges/2" für {no object} kann aus den folgenden Gründen nicht gefunden werden: Fehler beim Deserialisieren des JSON-RPC-Arguments mit dem Namen "ranges" und der Position 1 in den Typ "Paranext.DataProvider.Checks.CheckInputRange[]": end.ChapterNum must be > 0 (Parameter 'end')
[1]     at Object.request (C:\code\paranext-core\src\shared\services\network.service.ts:142:11)
[1]     at processTicksAndRejections (node:internal/process/task_queues:95:5) {
[1]   [cause]: 'JSON-RPC Request error -32602'
[1] }
[1] [/extension host error]

Also I got other errors on the console
a)

Warning: Encountered two children with the same key, `543391c6-f85f-48ba-c670-36f8361ec724`. Keys should be unique so that components maintain their identity across updates. Non-unique keys may cause children to be duplicated and/or omitted — the behavior is unsupported and could change in a future version.
    at div

b)

network.service.ts:142 Uncaught (in promise) Error: Die Methode "object:dotNetCheckRunner-data.setActiveRanges/2" für {no object} kann aus den folgenden Gründen nicht gefunden werden: Fehler beim Deserialisieren des JSON-RPC-Arguments mit dem Namen "ranges" und der Position 1 in den Typ "Paranext.DataProvider.Checks.CheckInputRange[]": end.ChapterNum must be > 0 (Parameter 'end')
    at Module.request (network.service.ts:142:11)Caused by: JSON-RPC Request error 0

c)

VM934 about:srcdoc:55523 Warning: Attempted to synchronously unmount a root while React was already rendering. React cannot finish unmounting the root until the current render has completed, which may lead to a race condition.

d)

index.html:1 Blocked aria-hidden on a <button> element because the element that just received focus must not be hidden from assistive technology users. Avoid using aria-hidden on a focused element or its ancestor. Consider using the inert attribute instead, which will also prevent focus. For more details, see the aria-hidden section of the WAI-ARIA specification at https://w3c.github.io/aria/#aria-hidden. 

<button type="button" class="dock-nav-more" tabindex="-1" aria-hidden="true" aria-haspopup="listbox" aria-controls="rc-tabs-6-more-popup" id="rc-tabs-6-more" aria-expanded="false" style="">...</button>

Code snippet:

[1] 13:43:36
[1] [extension host error]
[1] An unexpected error occurred while executing "object:platformScripture.checkAggregator-data.setActiveRanges" JSON-RPC method: Error: Die Methode "object:dotNetCheckRunner-data.setActiveRanges/2" für {no object} kann aus den folgenden Gründen nicht gefunden werden: Fehler beim Deserialisieren des JSON-RPC-Arguments mit dem Namen "ranges" und der Position 1 in den Typ "Paranext.DataProvider.Checks.CheckInputRange[]": end.ChapterNum must be > 0 (Parameter 'end')
[1]     at Object.request (C:\code\paranext-core\src\shared\services\network.service.ts:142:11)
[1]     at processTicksAndRejections (node:internal/process/task_queues:95:5) {
[1]   [cause]: 'JSON-RPC Request error -32602'
[1] }
[1] [/extension host error]

jolierabideau

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @Sebastian-ubs and @tjcouch-sil)

a discussion (no related file):

Previously, Sebastian-ubs wrote…

Hi Jolie, when trying I found some issues:

The project dropdown is for some reason disabled. This creates an unexpected combination of 3 colors with the bcv and the dropdown to the right. I cannot think of a reason, where we want it to be disabled
The last screenshot looks correct.
I am not able to persist any check configuration, (reported in https://paratextstudio.atlassian.net/browse/PT-2138, this does not only affect this pr)
ideally checks sidepanel should not allow to change the scrollgroup (like in PT9). until this is possible: Changing the scroll group fon the checks sidepanel should also change it for the project. Similarly when changing the scroll group on the project, it should update for the checks sidepanel.
On opening the checks sidepanel I got the following error:

[1] 13:43:36
[1] [extension host error]
[1] An unexpected error occurred while executing "object:platformScripture.checkAggregator-data.setActiveRanges" JSON-RPC method: Error: Die Methode "object:dotNetCheckRunner-data.setActiveRanges/2" für {no object} kann aus den folgenden Gründen nicht gefunden werden: Fehler beim Deserialisieren des JSON-RPC-Arguments mit dem Namen "ranges" und der Position 1 in den Typ "Paranext.DataProvider.Checks.CheckInputRange[]": end.ChapterNum must be > 0 (Parameter 'end')
[1]     at Object.request (C:\code\paranext-core\src\shared\services\network.service.ts:142:11)
[1]     at processTicksAndRejections (node:internal/process/task_queues:95:5) {
[1]   [cause]: 'JSON-RPC Request error -32602'
[1] }
[1] [/extension host error]

Also I got other errors on the console
a)

Warning: Encountered two children with the same key, `543391c6-f85f-48ba-c670-36f8361ec724`. Keys should be unique so that components maintain their identity across updates. Non-unique keys may cause children to be duplicated and/or omitted — the behavior is unsupported and could change in a future version.
    at div

b)

network.service.ts:142 Uncaught (in promise) Error: Die Methode "object:dotNetCheckRunner-data.setActiveRanges/2" für {no object} kann aus den folgenden Gründen nicht gefunden werden: Fehler beim Deserialisieren des JSON-RPC-Arguments mit dem Namen "ranges" und der Position 1 in den Typ "Paranext.DataProvider.Checks.CheckInputRange[]": end.ChapterNum must be > 0 (Parameter 'end')
    at Module.request (network.service.ts:142:11)Caused by: JSON-RPC Request error 0

c)

VM934 about:srcdoc:55523 Warning: Attempted to synchronously unmount a root while React was already rendering. React cannot finish unmounting the root until the current render has completed, which may lead to a race condition.

d)

index.html:1 Blocked aria-hidden on a <button> element because the element that just received focus must not be hidden from assistive technology users. Avoid using aria-hidden on a focused element or its ancestor. Consider using the inert attribute instead, which will also prevent focus. For more details, see the aria-hidden section of the WAI-ARIA specification at https://w3c.github.io/aria/#aria-hidden. 

<button type="button" class="dock-nav-more" tabindex="-1" aria-hidden="true" aria-haspopup="listbox" aria-controls="rc-tabs-6-more-popup" id="rc-tabs-6-more" aria-expanded="false" style="">...</button>

@Sebastian-ubs Thank you for testing this! I have seen these problems in development but wasn't seeing them when I published this PR, so I will take another look.

The project dropdown is disabled because that is what the issue said to do, I figured it was just temporary so that we could finish the rest of the configure options. It does work and I can enable it if that is what we want to do.
I commented on that issue, but I am currently working on this!
I feel it makes sense for this work to be in another issue, since my work in this PR didn't make this change. There is an issue PT-1524 for allowing web views to turn off the ability to change scroll group, which I believe would make this work-around unnecessary.

extensions/src/platform-scripture/contributions/localizedStrings.json line 56 at r1 (raw file):