💻 Preselect language filter based on user language #6044
Conversation
| @@ -37,10 +37,12 @@ | |||
| hx-target="#public_adventures" | |||
| value="">{{_('select_lang')}}</option> | |||
| {% for lang in available_languages %} | |||
| {% if lang %} | |||
There was a problem hiding this comment.
Wouldn't it be nicer to ensure that a None doesn't make it into this array in the Python code?
There was a problem hiding this comment.
agreed, it is filtered
| self.available_tags.update(adv_tags) | ||
| def update_lang_filter(self, adventures): | ||
| langs = [a['language'] for a in adventures if a.get('language')] | ||
| self.available_languages = set(langs) |
There was a problem hiding this comment.
It scares me a little that these seem to be instance members that go into a Flask template.
At best, they get overwritten on every request, and them being instance members is not necessary.
At worst, we write code expecting them to persist between requests and (because it works on a local dev setup) we ship that code and now the code behaves basically randomly on the production server.
Are we sure that these won't pollute into different requests?
boryanagoncharenko
force-pushed
the
adventures_5368
branch
from
403c358 to
8f40c24
Compare
|
Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork). |
|
Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork). |
This PR makes the following changes:
Fixes #5368
How to test
Ensure that you have public adventures for at least 2 languages (e.g. English and Dutch) and also adventures that do not have language set.
Please note that clearing the filter on tags is not possible right now. This seems like a bug which is not in the scope of the current task and perhaps should be addressed in the future. However, it will impact your testing.