-
Notifications
You must be signed in to change notification settings - Fork 2.3k
theme: move searx/templates/simple to searx/templates #1188
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
Conversation
|
@return42 FYI, it's a draft but the code won't change much (just fix found issue). |
| """Validates and saves preferences to cookies""" | ||
|
|
||
| def __init__(self, themes: List[str], categories: List[str], engines: Dict[str, Engine], plugins: Iterable[Plugin]): | ||
| def __init__(self, categories: List[str], engines: Dict[str, Engine], plugins: Iterable[Plugin]): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we really remove the theme option entirely. Maybe we want to support (external) themes one day?
I don't have a finial opinion, perhaps it is better after at all, to remove entirely first and think about a support of (external) themes when we have a need for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After thinking twice .. its more clear if we remove the theme from the preferences entirely / #1153 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also agree. For now we only have the simple theme and in the future we can think about external themes / a new impl. of themes as only CSS for example...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the master branch, to add a new theme, a maintainer creates a directory in the templates and static directories ... and then the existing theme (the simple theme) is forgotten ; which make sense from the maintainer point of view.
See my-spot fork for example:
- https://gitlab.e.foundation/e/infra/spot/-/commits/master/searx/templates/etheme
- https://spot.ecloud.global <-- the
themeparameter doesn't work.
With this PR, we make sure only the maintained theme is available.
The remaining question: how my-spot deals with its own theme? (my-spot is an example here)
- either reuse the
templatesandstaticdirectories. - either use two new directories, for example
searx/etheme/templatesandsearx/etheme/static:- it is easier to backport commits (no conflicts)
- the history of the
searx/ethemedirectory is clean.
I'm not sure this is a good solution, but this PR:
- make the code more straightforward / easier to understand for someone new to the project.
- enforce the fact we won't support more than one theme.
note: my-spot is based of searx anyway.
|
FYI this needs to be rebased on master before merge... Stuff like the |
169848b to
46e3024
Compare
81b5d4e to
c68e8f8
Compare
|
Before this PR, I would like to implement #1169 (comment) so the oscar's users can get back this layout. |
What does this PR do?
themepreferences is removed.the following line has to be comment out:searxng/tests/robot/test_webapp.py
Line 32 in 6db568b
Why is this change important?
How to test this PR locally?
Author's checklist
In another PR, the source files of the static directory will be moved to the
srcdirectory.The way to change the theme is going be in settings.yml:
searxng/searx/settings.yml
Lines 76 to 79 in 6db568b
The admin can have a second repository, then
static_pathandtemplates_pathcan reference this repository.Related issues