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

[mod] simple theme: smaller build #566

Merged
merged 2 commits into from Dec 1, 2021

Conversation

dalf
Copy link
Member

@dalf dalf commented Nov 30, 2021

What does this PR do?

remove:

  • searx/static/themes/simple/js/searxng.head.js
  • searx/static/themes/simple/js/searxng.js
  • searx/static/themes/simple/css/searxng-rtl.css
  • searx/static/themes/simple/css/searxng.css

These files are never used.
About the js files: the .map files references the sources instead of the concated version.

Why is this change important?

Remove some useless files from the built files.

Draft because:

How to test this PR locally?

  • make run, check Javascript is working (clear a search, image detail, map)

Author's checklist

Except searx/static/themes/simple/css/images, all files are now built from the sources.

Related issues

Related to #141

remove:
* searx/static/themes/simple/js/searxng.head.js
* searx/static/themes/simple/js/searxng.js
* searx/static/themes/simple/css/searxng-rtl.css
* searx/static/themes/simple/css/searxng.css

These files are never used.
About the js files: the .map files references the sources instead of the concated version.
@dalf dalf marked this pull request as draft November 30, 2021 22:13
@return42
Copy link
Member

return42 commented Dec 1, 2021

These files are never used.

I pushed 8212586 to remove this files from the static folder.

the .map files references the sources instead of the concated version.

this works, as long as we have the src folder underneath the /static path

"sources":["../src/js/main/00_toolkit.js","../src/js/main/keyboard.js", ...

To get #141 done, we need to copy these files to searx/static/themes/simple/js/

Draft because:

I don't think so, cleaning up the source files is IMO not related to a StyleGuide

  • the same thing can done for the Oscar theme.

No need for, don't lets waste time with this theme. We should make simple theme the default theme, then we can remove oscar theme and then we can assemble a build chain in /src that is straight forward and do no longer mess by the oscar theme.

With other words: oscar is in the way to have a clean build chain for client side stuff / making simple theme to the default (and removing oscar) should be prioritized.

remove:

    searx/static/themes/simple/js/searxng.js
    searx/static/themes/simple/js/searxng.head.js
    searx/static/themes/simple/css/searxng-rtl.css
    searx/static/themes/simple/css/searxng.css

Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
Copy link
Member

@return42 return42 left a comment

Choose a reason for hiding this comment

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

Hi @dalf, this PR animated me to add various improvements on top of branch. Please read through the commit messages and the comments from mine. Adopt what you think is good and drop what you don't think is good.

searx/static/themes/simple/gruntfile.js Outdated Show resolved Hide resolved
utils/lib_static.sh Outdated Show resolved Hide resolved
@return42
Copy link
Member

return42 commented Dec 1, 2021

@dalf is it OK, when we merge this PR? .. I have some patches in the loop where the merge of this PR is needed for.

@dalf dalf marked this pull request as ready for review December 1, 2021 19:29
@dalf dalf merged commit ae49b52 into searxng:master Dec 1, 2021
@dalf dalf deleted the simple-theme-smaller-build branch December 1, 2021 19:31
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

2 participants