Skip to content

Conversation

@duartegalvao
Copy link
Member

This PR makes it so that filters applied in editable lists (both the main editable list and the "get next editable") get stored in the browser's local storage. It also fixes a bug with external filters in which active filter options disappear when they become unavailable (such as a keyword no longer being present in the filtered search results).

@duartegalvao duartegalvao force-pushed the store-editabe-list-filters branch from 3f7b65e to 73e04c1 Compare February 21, 2024 10:29
@tomasr8
Copy link
Member

tomasr8 commented Feb 21, 2024

I wonder if there's a way to avoid the logic around loadedFilters and just initialize the state (filters, searchText) directly without the extra useEffect but I suppose not since we'll also need to call the event handlers (onChangeFilters) somewhere..

@duartegalvao duartegalvao force-pushed the store-editabe-list-filters branch from 73e04c1 to d60ffef Compare February 26, 2024 10:31
@duartegalvao
Copy link
Member Author

I wonder if there's a way to avoid the logic around loadedFilters and just initialize the state (filters, searchText) directly without the extra useEffect but I suppose not since we'll also need to call the event handlers (onChangeFilters) somewhere..

yeah, thats the whole problem. you'll always need a useEffect to call the handlers. there might be a way to avoid having that loadedFilters state but I can't think of any...

@ThiefMaster
Copy link
Member

I haven't really looked at the code in-depth yet, but can't we just call a function that saves the state in local storage when changing the state? Something like this:

onChangeWhatever={() => {
  updateWhateverState();
  saveToLocalStorage()
}}

@duartegalvao
Copy link
Member Author

I haven't really looked at the code in-depth yet, but can't we just call a function that saves the state in local storage when changing the state? Something like this:

onChangeWhatever={() => {
  updateWhateverState();
  saveToLocalStorage()
}}

we can do that, but we'll still need the useEffect to load the filters from storage, so I think it's simpler to also do the storing there all in one place...

@ThiefMaster
Copy link
Member

ThiefMaster commented Feb 26, 2024

a useEffect with an empty dependency list (since you'd only want to run this a single time) feels cleaner than very complex useEffect functions

@duartegalvao duartegalvao force-pushed the store-editabe-list-filters branch from a0b71d7 to c1e801d Compare February 27, 2024 09:38
This fixes the (existing) bug of selected options disappearing when they
become invalid, which could happen with external filters, and with this
PR it could also happen with internal filters if changes were made.
@ThiefMaster ThiefMaster force-pushed the store-editabe-list-filters branch from c1e801d to c43d173 Compare March 15, 2024 16:40
Copy link
Member

@ThiefMaster ThiefMaster left a comment

Choose a reason for hiding this comment

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

Seems to work fine, not really going to look deep into the filtering code :D

@ThiefMaster ThiefMaster enabled auto-merge (squash) March 15, 2024 16:42
@ThiefMaster ThiefMaster added this to the v3.3 milestone Mar 15, 2024
@ThiefMaster ThiefMaster merged commit e2819e4 into indico:master Mar 15, 2024
@ThiefMaster ThiefMaster deleted the store-editabe-list-filters branch March 15, 2024 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done 🚀

Development

Successfully merging this pull request may close these issues.

3 participants