Apply home current language to terms page#637
Conversation
|
Sorry for the delay! (You know why) Initial read through: this looks very clean, I just have to try it out (assigning to me to do so, when I am able)
This makes sense, and it prevents people from inadvertently changing their default language selection. Thank you very much @goodolcuppa , and again apologies for the delay. |
|
@goodolcuppa - small lint error. I'll re-run the PR to check the base-ci, it should be fine. |
|
Hi again @goodolcuppa -- sorry for falling off this, I've had some medical issues. The CI logs aren't available for this anymore, so I'm not sure what went wrong. The develop branch has updated a little bit since you pushed this, perhaps you could merge the develop branch into this work and repush, and we'll see where we're at. |
…home_current_language_to_terms_page
|
Thanks @goodolcuppa for reactivating the code, CI is running now and we'll see how it goes. Did you have other tweaks you wanted to make to this PR, or do you think it's all good to go? Cheers! edit: just the lint failure to be cleared up so far: |
|
It should be all good to go now, thanks |
jzohrab
left a comment
There was a problem hiding this comment.
Hi @goodolcuppa -- I just added a couple of questions for you. Cheers!
| let update_filter_storage = function() { | ||
| const store = { | ||
| filtDisplay: $('#filterControls').css('display'), | ||
| filtLanguage: $('#filtLanguage').val(), |
There was a problem hiding this comment.
Curious, why is this line no longer needed? We're not using storage for the language?
| dropdown.val(current_language_id); | ||
| const language_count = $("#filtLanguage option").length; | ||
| if (language_count == 1) { | ||
| dropdown.css("display", "none"); |
There was a problem hiding this comment.
Should it be hidden if there's only one language? I think it's ok to leave it. Thoughts?
Applying the current language filter from home page to the terms page
Closes #326
Uses the same value as home for the terms language filter. Changing the value on the terms page will not update the stored value, so on refresh, this will revert back to whichever value is set on the home page