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

Use title snippets if available #505

Merged
merged 4 commits into from
May 26, 2021
Merged

Use title snippets if available #505

merged 4 commits into from
May 26, 2021

Conversation

maneeshpm
Copy link
Contributor

@maneeshpm maneeshpm commented May 8, 2021

Fixes #82

With openzim/libzim#545 we now support snippet generation for titles. It can be used as the display label on the ui for highlighted titles via the "label" field. The old version used plain-title which is still available in the value field.

Changes included in this pr:

  • Sets label to title snippet if available
  • Set up proper rendering of snippet using _renderItem property of autocomplete ui

@codecov
Copy link

codecov bot commented May 8, 2021

Codecov Report

Merging #505 (1b56680) into master (b82fff9) will increase coverage by 0.02%.
The diff coverage is 100.00%.

❗ Current head 1b56680 differs from pull request most recent head 7b81795. Consider uploading reports for the commit 7b81795 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #505      +/-   ##
==========================================
+ Coverage   64.57%   64.60%   +0.02%     
==========================================
  Files          50       50              
  Lines        3562     3565       +3     
  Branches     1802     1805       +3     
==========================================
+ Hits         2300     2303       +3     
  Misses       1260     1260              
  Partials        2        2              
Impacted Files Coverage Δ
src/reader.cpp 44.04% <100.00%> (+0.20%) ⬆️
src/server/internalServer.cpp 87.05% <100.00%> (+0.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b82fff9...7b81795. Read the comment docs.

@maneeshpm maneeshpm marked this pull request as draft May 8, 2021 19:31
@maneeshpm
Copy link
Contributor Author

Screenshot from 2021-05-09 00-56-00
@veloman-yunkan The HTML seems to be getting escaped in the front end. How to fix this?

@kelson42
Copy link
Collaborator

kelson42 commented May 9, 2021

@MananJethwani Maybe you can help @maneeshpm on that one?

@MananJethwani
Copy link
Contributor

this I suppose is a problem with jquery-ui. because the data is received in taskbar.js as we expect it to be, and to stop this behavior we might have to use _renderItem property of autocomplete UI.

@maneeshpm
Copy link
Contributor Author

@MananJethwani
Copy link
Contributor

@maneeshpm jquery-ui.js is the library file from where we import suggestion system.

@maneeshpm
Copy link
Contributor Author

Thanks a lot for your help @MananJethwani, front end is not really my strong point 😅

Final result of the modification:
Screenshot from 2021-05-10 12-56-32

@kelson42 I believe this feature can be improved further by modifications in stemming and stopwords(openzim/libzim#324) to ensure all the matching terms are highlighted.

@kelson42
Copy link
Collaborator

@maneeshpm Great to see this implemented and great you see the connection to the other ticket :)

@maneeshpm maneeshpm marked this pull request as ready for review May 10, 2021 14:59
@maneeshpm maneeshpm requested a review from mgautierfr May 10, 2021 15:51
src/reader.cpp Outdated Show resolved Hide resolved
@maneeshpm maneeshpm changed the base branch from master to lizim_search_api_change May 16, 2021 05:33
@maneeshpm
Copy link
Contributor Author

Rebasing temporarily on #526

Base automatically changed from lizim_search_api_change to master May 17, 2021 13:06
@maneeshpm maneeshpm requested a review from mgautierfr May 17, 2021 14:10
include/reader.h Show resolved Hide resolved
@maneeshpm maneeshpm requested a review from mgautierfr May 18, 2021 18:49
@kelson42
Copy link
Collaborator

@maneeshpm rebasing maybe?

@kelson42
Copy link
Collaborator

@mgautierfr I have rebased this branch. We should really review/merge.

With openzim/libzim#545 we now support snippet generation of titles
which can be used as the display label on the ui for highlighted titles
via the "label" field.
The old version used plain title which is still available in the value
field.
To render the snippets properly, we need to use the _renderItem property
of the autocomple ui.
This is a helper class that allows to create and manage individual
suggestion item and their data.
Each sugestions used to be stored as vector of strings to hold various values
such as title, path etc inside them. With this commit, we use the new
dedicated class `SuggestionItem` to do the same.
@mgautierfr
Copy link
Member

Rebased-fixup on master (and fix conflicts). We are good.

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.

Snippets for suggestions should return highlighted title, not content.
4 participants