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

[simple theme] SearXNG wordmark & favicon #473

Merged
merged 6 commits into from
Nov 13, 2021

Conversation

return42
Copy link
Member

@return42 return42 commented Nov 3, 2021

What does this PR do?

[simple theme] SearXNG wordmark & favicon

Why is this change important?

"the favicon is not really suited for dark mode (basically we need different icons for dark and for light mode eventually)"

Author's checklist

Favicon (see tab) and wordmark (left to the search form) has been changed ..

grafik

before this PR:

image

Related issues

#378

@@ -1,6 +1,6 @@
<form id="search" method="{{ method or 'POST' }}" action="{{ url_for('search') }}">
<a id="search_logo" href="{{ url_for('index') }}">
<img class="search_logo_img" src="{{ url_for('static', filename='img/favicon.png') }}">
<img class="search_logo_img" src="{{ url_for('static', filename='img/searxng-wordmark.svg') }}">
Copy link
Member

@dalf dalf Nov 3, 2021

Choose a reason for hiding this comment

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

To avoid an additional HTTP request, it can be replaced by an optimized inline version of the SVG file (429 bytes) :

<svg viewBox="0 0 92 92" version="1.1"><g transform="translate(-40.92 -17.42)"><circle cx="75.92" cy="53.9" r="30" fill="none" stroke="#3050ff" stroke-width="10"/><path d="M67.51 37.92a18 18 0 0 1 21.06 3.3 18 18 0 0 1 3.13 21.09" fill="none" stroke="#3050ff" stroke-width="5"/><rect width="18.85" height="39.96" x="3.71" y="122.09" ry="0" transform="rotate(-46.23)" fill="#3050ff"/></g></svg>

This version can be an external file that is included using the Jinja2 syntax.

The version above was made using https://jakearchibald.github.io/svgomg/ with these options:

  • set precision to 2
  • check "prefer viewbox to width/height" (at the bottom of the options)
  • Manually remove xmlns="http://www.w3.org/2000/svg".

For reference:


[EDIT] Also, the PNG file size can be optimized and reduced to 1.2KB.

Copy link
Member Author

Choose a reason for hiding this comment

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

This version can be an external file that is included using the Jinja2 syntax.

I assume you are talking about something like this ... ?

from flask import Markup
---
    return render(
        'results.html',
        wordmark_svg = Markup(open('.....img/searxng-wordmark.svg').read)

To avoid an additional HTTP request

Yes, we can avoid one request, but on the opposite (if I'm not wrong) we lost flexibility with the static/themes/simple folder

ui:
# Custom static path - leave it blank if you didn't change
static_path: ""

Copy link
Member

@dalf dalf Nov 3, 2021

Choose a reason for hiding this comment

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

What about

webapp.py:

wordmark_contents = {}
for indice, theme in enumerate(themes):
    ...
    workmark_file = os.path.join(settings['ui']['static_path'], 'theme', theme, 'img/searxng-wordmark.svg')
    if os.path.isfile(workmark_file):
        with os.open(workmark_file, 'rb') as f:
            wordmark_contents[theme] = f.read()
    ...


def render(...):
  ...
  kwargs['wordmark_content'] = wordmark_contents.get(kwargs['theme'])
  ...

search.html:

{{ wordmark_content }}

?

[EDIT] Too complicated.

Perhaps this:

  • add a new function : content_for, same as url_for but actually returns the content.
  • kwargs['content_for'] = content_for

--> {{ content_for('img/searxng-wordmark.svg') }}

Copy link
Member Author

Choose a reason for hiding this comment

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

@dalf I have tried what you suggested, see dd4da6f / but now I have a problem to size the inline SVG, see
dd4da6f#r743818450

@return42 return42 force-pushed the searxng-wordmark branch 2 times, most recently from b6c9f6c to 5cb20c6 Compare November 5, 2021 16:33
Comment on lines 3 to 5
<img class="search_logo_img" src="{{ url_for('static', filename='img/searxng-wordmark.svg') }}">
<div class="search_logo_img">
{{ insert_xml('img/searxng-wordmark.svg') }}
</div>
Copy link
Member Author

Choose a reason for hiding this comment

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

this leads into ..

grafik

and I do not know how I can set a fixed size for the <div> container or how the can set a fixed box-size for the inline SVG ..

I'm not an CSS expert and I haven't understand the CSS box model really.

@dalf
Copy link
Member

dalf commented Nov 6, 2021

I've pushed another simpler solution (it requires to build the themes):

  • {% include '__common__/logo.svg' without context %} (without context so the result is cached).
  • __common__/logo.svg is the minified version (perhaps it should be simple/logo.svg ? )

So, there is no need for a special function that read the .svg at each request.

@mrpaulblack
Copy link
Member

I suggest the following (adding the margin: 0.5rem 0 auto 0; to the svg selector):

#search_logo {
  grid-area: logo;
  display: flex;
  align-items: center;
  justify-content: center;

  svg {
    flex: 1;
    width: 30px;
    height: 30px;
    margin: 0.5rem 0 auto 0;
  }
}

(it makes sure that the logo is always displayed next to the search input field)
On tablet it looks like that with the fix:
image

return42 added a commit to return42/searxng that referenced this pull request Nov 8, 2021
Suggested-by: @dalf searxng#473 (comment)
Suggested-by: @mrpaulblack searxng#473 (comment)
Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
return42 added a commit to return42/searxng that referenced this pull request Nov 8, 2021
Suggested-by: @dalf searxng#473 (comment)
Suggested-by: @mrpaulblack searxng#473 (comment)
Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
@return42
Copy link
Member Author

return42 commented Nov 8, 2021

@dalf @mrpaulblack: thanks a lot for your helping comments, I placed what you suggested in commit:

  • fc00cb1 [simple theme] replace old searx logo by searxng-wordmark.min.svg

Adittionally make static.build.commit now builds additional files:

  • the minified SVG is generated by html-minifier in commit dc02b17
  • simple/img/favicon.png is generated by convert_if_newer() in commit 4a01683

In 21e690c I optimized the origin searxng-wordmark.svg and a792d5f builds minified searxng-wordmark.min.svg and converts favicon.png (see svg + png files from build).

Copy link
Member

@mrpaulblack mrpaulblack left a comment

Choose a reason for hiding this comment

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

👎 The favicon.png does not have a alpha channel...
image

Is this intended? (Or to rephrase: is it possible to generate favicon.png with an alpha channel from the svg?)

Everything else LGTM:
👍 the make themes.all and make themes.simple exec the htmlmin step for simple properly
👍 watermark displays correctly on desktop, tablet and mobile
👍 the jinja2 template inlines the svg correctly into the html

return42 added a commit to return42/searxng that referenced this pull request Nov 9, 2021
Suggested-by: @dalf searxng#473 (comment)
Suggested-by: @mrpaulblack searxng#473 (comment)
Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
manage Outdated
local src="searx/static/themes/simple/src"
local static="searx/static/themes/simple"
( set -e
convert_if_newer "$src/svg/searxng-wordmark.svg" "$static/img/favicon.png" -background none -resize 64x64
Copy link
Member Author

Choose a reason for hiding this comment

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

👎 The favicon.png does not have a alpha channel... image Is this intended? (Or to rephrase: is it possible to generate favicon.png with an alpha channel from the svg?)

Oops .. it is fixed right here by adding -background none to the convert command.

Copy link
Member

Choose a reason for hiding this comment

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

I wish to keep all the build steps inside gruntfiles.js

In the simple theme directory, npm run watch allows to rebuild the theme a files change. If we mix bash and node it starts to be complicated to do that.

There are some grunt modules to convert to svg to png, but I haven't tried them.

Otherwise it is possible to call a command line from grunt: https://www.npmjs.com/package/grunt-exec

But, the logo won't change as often the other files, so IMO, it is not an issue if there is automatic build of the favicon.

Copy link
Member Author

Choose a reason for hiding this comment

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

I wish to keep all the build steps inside gruntfiles.js

Hm, .. I understand to want to have all theme build-tasks at one place (grunt), but the drawback I see is; with grunt-exec we can't express a conditional convert if origin is newer. IMO calling shell tasks from grunt increase complexity unneeded.

@@ -8,6 +8,8 @@
"grunt-contrib-jshint": "~3.1.1",
"grunt-contrib-less": "~3.0.0",
"grunt-contrib-uglify": "~5.0.1",
"grunt-xmlmin": "~0.1.8",
"grunt-contrib-htmlmin": "~3.1.0",
Copy link
Member

Choose a reason for hiding this comment

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

grunt-image uses svgo. The min.svg file size can be reduced by ~ half (it removes the sodipodi elements for example).

Copy link
Member Author

Choose a reason for hiding this comment

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

OK I understand .. can we optimize this later? First I think we should clarify which version of svgo we should use. ATM we have an issue reported when make node.clean node.env is build:

npm WARN deprecated svgo@0.6.6: This SVGO version is no longer supported. Upgrade to v2.x.x.

This issue is caused by grunt-webfont and might be also related to #488 (comment) / Hint: https://github.com/sapegin/grunt-webfont is not maintained since years, see archived repository at https://github.com/sapegin/grunt-webfont and we have other issues with, see #248 (comment)

manage Outdated
local src="searx/static/themes/simple/src"
local static="searx/static/themes/simple"
( set -e
convert_if_newer "$src/svg/searxng-wordmark.svg" "$static/img/favicon.png" -background none -resize 64x64
Copy link
Member

Choose a reason for hiding this comment

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

I wish to keep all the build steps inside gruntfiles.js

In the simple theme directory, npm run watch allows to rebuild the theme a files change. If we mix bash and node it starts to be complicated to do that.

There are some grunt modules to convert to svg to png, but I haven't tried them.

Otherwise it is possible to call a command line from grunt: https://www.npmjs.com/package/grunt-exec

But, the logo won't change as often the other files, so IMO, it is not an issue if there is automatic build of the favicon.

return42 added a commit to return42/searxng that referenced this pull request Nov 12, 2021
Suggested-by: @dalf searxng#473 (comment)
Suggested-by: @mrpaulblack searxng#473 (comment)
Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
@return42
Copy link
Member Author

@dalf @mrpaulblack I rebased this PR and gave some feedback to your requests. What do you think, is this PR ready to merge or is there anything left to do?

@mrpaulblack
Copy link
Member

LGTM 👍

@dalf
Copy link
Member

dalf commented Nov 12, 2021

The favicon.png does not have an alpha channel:
https://github.com/searxng/searxng/blob/7ea3faf033b94de82eefd1a92cebfe9241735945/searx/static/themes/simple/img/favicon.png


With convert "ImageMagick 6.9.10-23 Q16 x86_64 20190101 https://imagemagick.org" (Ubuntu 20.04), the rendered image is broken.

return42 added a commit to return42/searxng that referenced this pull request Nov 12, 2021
Suggested-by: @dalf searxng#473 (comment)
Suggested-by: @mrpaulblack searxng#473 (comment)
Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
Comment on lines +698 to +699
convert_if_newer "$src/svg/searxng-wordmark.svg" "$static/img/favicon.png" \
-transparent white -resize 64x64
Copy link
Member Author

@return42 return42 Nov 12, 2021

Choose a reason for hiding this comment

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

The favicon.png does not have an alpha channel:

@dalf thanks a lot for your attention! ... I'm very sorry for the mess .. it is not ImageMagick what is broken .. it is me 🤦🏻

Its now fixed ...

grafik

Copy link
Member

@mrpaulblack mrpaulblack Nov 13, 2021

Choose a reason for hiding this comment

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

LGTM 👍
image

Related-to:

- searxng#430 (comment)
- searxng#378

Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
Command::

  ./node_modules/.bin/html-minifier \
      --remove-comments \
      --collapse-whitespace \
      src/svg/searxng-wordmark.svg \
      -o ../../../templates/__common__/searxng-wordmark.min.svg

- html-minifier: https://github.com/kangax/html-minifier
  - onilne: https://kangax.github.io/html-minifier
  - grunt: https://www.npmjs.com/package/grunt-contrib-htmlmin
  - grunt-contrib-htmlmin: https://github.com/gruntjs/grunt-contrib-htmlmin
  - npm: https://www.npmjs.com/package/html-minifier

To test, rebuild your node environment::

  make node.env

Alternatives:

- pretty-data: https://github.com/vkiryukhin/pretty-data
  - grunt: https://www.npmjs.com/package/grunt-xmlmin
  - grunt-xmlming: https://github.com/dtrunk90/grunt-xmlmin
  - npm: https://www.npmjs.com/package/grunt-xmlmin

- minify-xml: https://github.com/kristian/minify-xml
  - no grunt package available
  - npm: https://www.npmjs.com/package/minify-xml

src/svg/searxng-wordmark.svg':
'../../../templates/__common__/searxng-wordmark.min.svg'

Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
new bash function convert_if_newer() / usage::

    convert_if_newer <origfile> <outfile> [<options>, ...]
    convert_if_newer "path/to/origin.svg" "path/to/converted.png" -transparent white -resize 64x64

Run's ImageMagik' convert comand to generate <outfile> from <origfile>, if
<origfile> is newer than <outfile>.  The command line is to convert is::

    convert <origfile> [<options>, ...] <outfile>

PNG 'searx/static/themes/simple/img/favicon.png' has been created by::

  $ make themes.simple
  CONVERT   searx/static/themes/simple/src/svg/searxng-wordmark.svg -transparent white -resize 64x64 searx/static/themes/simple/img/favicon.png
  ...

Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
Suggested-by: @dalf searxng#473 (comment)
Suggested-by: @mrpaulblack searxng#473 (comment)
Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
Remove XML namespaces from Incscape [1]::

   xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd"
   xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape"

[1] https://wiki.inkscape.org/wiki/PlainSVG

Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
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.

None yet

3 participants