Skip to content

Got rid of static/templates/no_search_result.html #744

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

Merged
merged 2 commits into from
Apr 6, 2022

Conversation

veloman-yunkan
Copy link
Collaborator

This follow-up to #738 is probably the last step before removing the WIP status from #679.

@veloman-yunkan
Copy link
Collaborator Author

I extracted these two commits into a PR of their own because they interfere with #712. In #712 (review) @mgautierfr asked a question which proved to be prophetic:

  • I wonder if we should also store the md5id somewhere in the cpp code if we have to generate links to resource on the cpp side. I don't know if it is really useful, just questioning.

As a result of unifying various error templates, now we have to generate a link to a static resource from C++ code (see 53f5a3d#diff-9e3da86d6232708bc11700bf2326552f6ad16b40c029c3ac138470bf4e7d9be7R602). At this point the easiest solution seems to preserve no_search_result.html as a separate template. I have published this PR so that we can discuss it together with #712.

Copy link
Member

@mgautierfr mgautierfr left a comment

Choose a reason for hiding this comment

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

We should just change the details message but else we are good.

The question about the md5id in cpp is interesting but this PR is working as it is, and we should discuss about this in #712. So this is not a blocker for this PR.

@codecov
Copy link

codecov bot commented Apr 6, 2022

Codecov Report

Merging #744 (6e79a66) into master (c1823b8) will increase coverage by 0.48%.
The diff coverage is 100.00%.

❗ Current head 6e79a66 differs from pull request most recent head ae1bf39. Consider uploading reports for the commit ae1bf39 to get more accurate results

@@            Coverage Diff             @@
##           master     #744      +/-   ##
==========================================
+ Coverage   59.98%   60.47%   +0.48%     
==========================================
  Files          56       56              
  Lines        3724     3724              
  Branches     2057     2058       +1     
==========================================
+ Hits         2234     2252      +18     
+ Misses       1489     1471      -18     
  Partials        1        1              
Impacted Files Coverage Δ
src/server/response.h 100.00% <ø> (ø)
src/server/internalServer.cpp 82.28% <100.00%> (+1.47%) ⬆️
src/server/response.cpp 90.99% <100.00%> (+0.04%) ⬆️
src/tools/lrucache.h 85.29% <0.00%> (+14.70%) ⬆️
src/tools/concurrent_cache.h 100.00% <0.00%> (+33.33%) ⬆️

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 c1823b8...ae1bf39. Read the comment docs.

The "Fulltext search unavailable" error page is now generated using the
static/templates/error.html template. Also added two test cases checking
that error page.
@mgautierfr mgautierfr force-pushed the fullsearch_text_unavailable_error branch from 6e79a66 to ae1bf39 Compare April 6, 2022 12:42
@mgautierfr
Copy link
Member

Rebased/fixup on master

@mgautierfr mgautierfr merged commit a17258f into master Apr 6, 2022
@mgautierfr mgautierfr deleted the fullsearch_text_unavailable_error branch April 6, 2022 13:14
veloman-yunkan added a commit that referenced this pull request May 8, 2022
In #727 inline CSS [was extracted](e4a4b2f)
from `static/templates/no_search_result.html` into a separate stylesheet
resource. The purpose was to later

1. get rid of the custom `static/templates/no_search_result.html` error
   template and use a general purpose error template instead (this was
   accomplished by PR #744).

2. deduplicate the CSS code between `static/templates/no_search_result.html` and
   `static/templates/search_result.html` by making the latter to also refer to
   an internal CSS resource rather than containing inline stylesheet code.

While preparing to implement the 2nd point, I figured out that
`kiwix::SearchRenderer` is used as a component in `kiwix-desktop` too,
which probably would be upset by a link to a libkiwix's internal CSS resource.

This commit documents that finding.
veloman-yunkan added a commit that referenced this pull request May 10, 2022
In #727 inline CSS [was extracted](e4a4b2f)
from `static/templates/no_search_result.html` into a separate stylesheet
resource. The purpose was to later

1. get rid of the custom `static/templates/no_search_result.html` error
   template and use a general purpose error template instead (this was
   accomplished by PR #744).

2. deduplicate the CSS code between `static/templates/no_search_result.html` and
   `static/templates/search_result.html` by making the latter to also refer to
   an internal CSS resource rather than containing inline stylesheet code.

While preparing to implement the 2nd point, I figured out that
`kiwix::SearchRenderer` is used as a component in `kiwix-desktop` too,
which probably would be upset by a link to a libkiwix's internal CSS resource.

This commit documents that finding.
veloman-yunkan added a commit that referenced this pull request May 15, 2022
In #727 inline CSS [was extracted](e4a4b2f)
from `static/templates/no_search_result.html` into a separate stylesheet
resource. The purpose was to later

1. get rid of the custom `static/templates/no_search_result.html` error
   template and use a general purpose error template instead (this was
   accomplished by PR #744).

2. deduplicate the CSS code between `static/templates/no_search_result.html` and
   `static/templates/search_result.html` by making the latter to also refer to
   an internal CSS resource rather than containing inline stylesheet code.

While preparing to implement the 2nd point, I figured out that
`kiwix::SearchRenderer` is used as a component in `kiwix-desktop` too,
which probably would be upset by a link to a libkiwix's internal CSS resource.

This commit documents that finding.
veloman-yunkan added a commit that referenced this pull request May 15, 2022
In #727 inline CSS [was extracted](e4a4b2f)
from `static/templates/no_search_result.html` into a separate stylesheet
resource. The purpose was to later

1. get rid of the custom `static/templates/no_search_result.html` error
   template and use a general purpose error template instead (this was
   accomplished by PR #744).

2. deduplicate the CSS code between `static/templates/no_search_result.html` and
   `static/templates/search_result.html` by making the latter to also refer to
   an internal CSS resource rather than containing inline stylesheet code.

While preparing to implement the 2nd point, I figured out that
`kiwix::SearchRenderer` is used as a component in `kiwix-desktop` too,
which probably would be upset by a link to a libkiwix's internal CSS resource.

This commit documents that finding.
veloman-yunkan added a commit that referenced this pull request May 17, 2022
In #727 inline CSS [was extracted](e4a4b2f)
from `static/templates/no_search_result.html` into a separate stylesheet
resource. The purpose was to later

1. get rid of the custom `static/templates/no_search_result.html` error
   template and use a general purpose error template instead (this was
   accomplished by PR #744).

2. deduplicate the CSS code between `static/templates/no_search_result.html` and
   `static/templates/search_result.html` by making the latter to also refer to
   an internal CSS resource rather than containing inline stylesheet code.

While preparing to implement the 2nd point, I figured out that
`kiwix::SearchRenderer` is used as a component in `kiwix-desktop` too,
which probably would be upset by a link to a libkiwix's internal CSS resource.

This commit documents that finding.
veloman-yunkan added a commit that referenced this pull request May 18, 2022
In #727 inline CSS [was extracted](e4a4b2f)
from `static/templates/no_search_result.html` into a separate stylesheet
resource. The purpose was to later

1. get rid of the custom `static/templates/no_search_result.html` error
   template and use a general purpose error template instead (this was
   accomplished by PR #744).

2. deduplicate the CSS code between `static/templates/no_search_result.html` and
   `static/templates/search_result.html` by making the latter to also refer to
   an internal CSS resource rather than containing inline stylesheet code.

While preparing to implement the 2nd point, I figured out that
`kiwix::SearchRenderer` is used as a component in `kiwix-desktop` too,
which probably would be upset by a link to a libkiwix's internal CSS resource.

This commit documents that finding.
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.

2 participants