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.

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

Jump to bottom

156 searchwindow use edition path for collection function #463

Merged
merged 9 commits into from
Dec 2, 2024
Merged

156 searchwindow use edition path for collection function #463

merged 9 commits into from
Dec 2, 2024

Conversation

hizclick
Copy link
Contributor

@hizclick hizclick commented Nov 7, 2024 •
edited by krHERO
Loading

Description, Context and related Issue

Refs #

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have updated the documentation accordingly.
  • All new and existing tests passed.

@hizclick hizclick linked an issue Nov 7, 2024 that may be closed by this pull request
SearchWindow: Use edition path for collection function #156
Closed
@peterstadler
Copy link
Member

This still needs some work. There's inline function documentation missing and the kwic results are not output properly, i.e. only the search terms without context.
Additionally, the text input in the upper right does not trigger a search but only opens the search window. That should be fixed, too.

I will look into these issues.

@peterstadler peterstadler marked this pull request as draft November 8, 2024 07:49
@peterstadler peterstadler self-assigned this Nov 8, 2024
@peterstadler peterstadler force-pushed the 156-searchwindow-use-edition-path-for-collection-function branch from ea41f9c to b23fc14 Compare November 8, 2024 14:53
@peterstadler
Copy link
Member

I think I fixed the remaining points (function documentation, KWIC, top bar text input).

There's yet another thing, i.e. the CSS styling which seems to be missing. Neither the KWIC display seems adequate nor are the search terms highlighted in the text (although the className searchResult is injected correctly). This seems related to #451 so I wonder if @bwbohl can reproduce the issue here?

Some Screenshots to illustrate the CSS issue:

Bildschirmfoto 2024-11-08 um 16 25 31 Bildschirmfoto 2024-11-08 um 16 26 51

@peterstadler peterstadler marked this pull request as ready for review November 8, 2024 15:27
@peterstadler peterstadler requested review from roewenstrunk and removed request for peterstadler November 8, 2024 15:28
@peterstadler peterstadler added this to the 1.0.0 milestone Nov 8, 2024
@peterstadler peterstadler force-pushed the 156-searchwindow-use-edition-path-for-collection-function branch from e77fa3c to 0ad3949 Compare November 10, 2024 10:24
@peterstadler peterstadler mentioned this pull request Nov 11, 2024
Closed
peterstadler and others added 8 commits November 20, 2024 09:19
@peterstadler
first shot at fixing #156
f83dab3
@peterstadler
that's actually not needed
82f413f 
The `edition` parameter is automatically added by `window.doAJAXRequest`
@peterstadler
move variables to query body
3e40ec7 
and fix parameter name from `uri` to `edition`
@hizclick @peterstadler
fix function for displaying kwic results
e6cdc9f
@peterstadler
improve inline function documentation
d83133d
@peterstadler
fix callback function
8307eb4
@peterstadler
directly do search
44d14c7 
when query terms are entered via the top bar text input
@peterstadler
order imports and declarations
eb5c7f4 
according to the Style Guide
@peterstadler peterstadler force-pushed the 156-searchwindow-use-edition-path-for-collection-function branch from cc547a6 to eb5c7f4 Compare November 20, 2024 08:19
@peterstadler
Copy link
Member

pinging @roewenstrunk and @bwbohl to review

roewenstrunk
roewenstrunk approved these changes Dec 2, 2024
View reviewed changes
Copy link
Member

@roewenstrunk roewenstrunk left a comment

Choose a reason for hiding this comment

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

Tested with Klarinettenquintett

@roewenstrunk roewenstrunk merged commit 6e330cd into develop Dec 2, 2024
4 checks passed
@roewenstrunk roewenstrunk deleted the 156-searchwindow-use-edition-path-for-collection-function branch December 2, 2024 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roewenstrunk roewenstrunk roewenstrunk approved these changes

@bwbohl bwbohl Awaiting requested review from bwbohl

Assignees

@peterstadler peterstadler

Labels
None yet
Projects
Edirom Development
Status: Done
Milestone
1.0.0
Development

Successfully merging this pull request may close these issues.

SearchWindow: Use edition path for collection function
3 participants
@hizclick @peterstadler @roewenstrunk