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

Refactor: Move tag suggestions into Redux #1996

Merged
merged 1 commit into from
Apr 14, 2020

Conversation

dmsnell
Copy link
Member

@dmsnell dmsnell commented Apr 14, 2020

Since we introduced tag suggestions in #1648 we have been recalculating
the list of tag suggestions on every render, at least twice (once to get
the count of tag suggestions in note-list and once in tag-suggestions).

In order to prevent needless re-rendering we implemented custom memoization
in the tag-suggestions component.

In this patch we're moving the tag suggestions logic into our search module
where the tag suggestions can update with the rest of the search results.
By doing this we'll leave one place to update them, couple them with the
filtered notes, and be able to remove our custom memoizatoin without losing
the benefits it brought us.

@dmsnell dmsnell added this to the 1.16 milestone Apr 14, 2020
@dmsnell dmsnell requested a review from a team April 14, 2020 01:54
@dmsnell dmsnell force-pushed the refactor/tag-suggestions-in-redux branch from 1fb6531 to 90a0ddc Compare April 14, 2020 02:04
Since we introduced tag suggestions in #1648 we have been recalculating
the list of tag suggestions on every render, at least twice (once to get
the count of tag suggestions in note-list and once in tag-suggestions).

In order to prevent needless re-rendering we implemented custom memoization
in the tag-suggestions component.

In this patch we're moving the tag suggestions logic into our search module
where the tag suggestions can update with the rest of the search results.
By doing this we'll leave one place to update them, couple them with the
filtered notes, and be able to remove our custom memoizatoin without losing
the benefits it brought us.
@dmsnell dmsnell force-pushed the refactor/tag-suggestions-in-redux branch from 90a0ddc to d178f15 Compare April 14, 2020 02:05
Copy link
Member

@codebykat codebykat left a comment

Choose a reason for hiding this comment

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

Code makes sense and tests fine 👍

@dmsnell dmsnell merged commit c3b6200 into develop Apr 14, 2020
@dmsnell dmsnell deleted the refactor/tag-suggestions-in-redux branch April 14, 2020 02:29
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.

2 participants