Skip to content
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

161 search by notation #269

Merged
merged 37 commits into from
Sep 7, 2023
Merged

161 search by notation #269

merged 37 commits into from
Sep 7, 2023

Conversation

sroertgen
Copy link
Contributor

The labels from

  • skos:altLabel
  • skos:notation
  • skos:hiddenLabel
  • skos:definition

are now indexed. By selection of the respective label, only the relevant index gets loaded. This means we don't have one blown up index, but multiple for each label, which makes us handle even quite large vocabs performant.
Also flexsearch has quite improved, so the index is not so big like before.

Re: Accessibility you are now able to check and uncheck boxes using Alt + <first letter of label e.g. Alt + a to toggle search for altLabels. Ctrl + k will you bring back to the search box.

Would like to know if the remaining labels i asked for in #128 should also be added. Since they will not blow up the index by default, I see no problem in doing so.

Will close #128 #161

@sroertgen sroertgen linked an issue Aug 31, 2023 that may be closed by this pull request
@sroertgen sroertgen requested a review from acka47 August 31, 2023 11:54
@sroertgen sroertgen self-assigned this Aug 31, 2023
@sroertgen sroertgen linked an issue Aug 31, 2023 that may be closed by this pull request
@sroertgen
Copy link
Contributor Author

The failing tests, I might handle in a separate issue. Everything goes through locally and I suspect some GitHub Action related issue, since sometimes one cypress test fails and sometimes two.

@sroertgen sroertgen removed the request for review from acka47 August 31, 2023 11:57
@sroertgen
Copy link
Contributor Author

ok, I'm giving up with the cypress GitHub Action thing. Local tests work. I will disable this action for now and open an issue for it.

@sroertgen sroertgen requested a review from acka47 August 31, 2023 13:08
conceptSchemeId was sometimes still having the old value.
This led to unneccessary double rendering.
Removal seems to improve performance and reduce errors with fetching the wrong index
(cause the old conceptSchemeId was still hanging around).
I improved testing and also the components. Maybe there was something I overlooked before.
Still don't know why it works locally, but fails in the action.
Let's see if this is still the case.
@sroertgen
Copy link
Contributor Author

@acka47 you can test now, e.g. https://test.skohub.io/sroertgen/test-vocabs/heads/main/w3id.org/two-concepts-one-file/c1.de.html

or you can send your own vocab to the test-server

@sroertgen
Copy link
Contributor Author

Telephone protocol:
The searchable attributes should not be directly visible, but should be configurable inside a modal. The modal can be toggled by a button in the search box.

@sroertgen
Copy link
Contributor Author

sroertgen commented Sep 4, 2023

@acka47 please check again.
Attributes are now set in a modal.

@acka47
Copy link
Member

acka47 commented Sep 5, 2023

This looks really good. One last UX thing: I find myself adjusting toggle buttons and then trying to click into the search window to start a search. This won't work at the moment and I have to go back to hit the "close" button first. There are two options for improvements:

  1. Do not require to hit the "close" button and also close the window when you click anywhere else.
  2. Remove the "close" button completely.

Copy link
Member

@acka47 acka47 left a comment

Choose a reason for hiding this comment

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

Please adjust the UX as commented

@acka47 acka47 removed their assignment Sep 5, 2023
@sroertgen
Copy link
Contributor Author

@acka47 Outside closing is now implemented.
Please check again, e.g. https://test.skohub.io/sroertgen/test-vocabs/heads/main/w3id.org/two-concepts-one-file/c1.de.html

Copy link
Member

@acka47 acka47 left a comment

Choose a reason for hiding this comment

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

+1

@sroertgen sroertgen merged commit c0a59e7 into main Sep 7, 2023
2 checks passed
@sroertgen sroertgen deleted the 161-search-by-notation branch September 7, 2023 12:49
@sroertgen sroertgen restored the 161-search-by-notation branch September 8, 2023 14:44
@sroertgen sroertgen deleted the 161-search-by-notation branch February 7, 2024 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Search by notation Add strings from skos:altLabel & skos:example to FlexSearch index
2 participants