WIP: Types/type note list #1807

dmsnell · 2020-01-04T03:52:16Z

This patch is a little wiry: I'm going to split it into a million little PRs.

Please ignore.

In the existing code we're recomputing the current list item in the note list when we've already assigned it above. In this patch we're using the already-assigned value. This is part of broader work to separate modifying the search parameters from actually filtering the notes and was originally created as part of an exploratory PR #1807.

Previously we have been (successfully) passing string values of `0` into the `tabIndex` property on parts of the note list preview. While acceptible this is the wrong datatype. In this patch we're passing the `{0}` value instead through React so that the types unify. This is part of broader work to separate modifying the search parameters from actually filtering the notes and was originally created as part of an exploratory PR #1807.

Keeping up with React standards. This patch replaces the use of a ref function with a `createRef()`. This will end up making our types easier. This is part of broader work to separate modifying the search parameters from actually filtering the notes and was originally created as part of an exploratory PR #1807.

When adding tag-suggestions in the note list we create a "composite list" of notes and placeholders. The placeholders designate where headers will go, where the tag suggestions list will go, and where a message will go if there are no matching notes. There were two issues with this structure: - it was difficult to type because of how its structure overlapped a `NoteEntity` but didn't have a key field for discriminating the type. everything works just fine but the type signatures become harder to write. - we were storing display information in the `data` property and that, as a render concern, wasn't necessary Further, we were directly mutating `notes` which came from `filterNotes()` and the Redux state. We want to avoid directly mutating values we receive from other places in order to keep data flows clear and obvious. In this patch we are replacing the object data type for placeholders with simpler string counterparts, moving the label text into the `render` function, and exchanging a mutating operation for a non-mutating one. This is part of broader work to separate modifying the search parameters from actually filtering the notes and was originally created as part of an exploratory PR #1807.

In the existing code we're recomputing the current list item in the note list when we've already assigned it above. In this patch we're using the already-assigned value. This is part of broader work to separate modifying the search parameters from actually filtering the notes and was originally created as part of an exploratory PR #1807.

Previously we have been (successfully) passing string values of `0` into the `tabIndex` property on parts of the note list preview. While acceptible this is the wrong datatype. In this patch we're passing the `{0}` value instead through React so that the types unify. This is part of broader work to separate modifying the search parameters from actually filtering the notes and was originally created as part of an exploratory PR #1807.

When adding tag-suggestions in the note list we create a "composite list" of notes and placeholders. The placeholders designate where headers will go, where the tag suggestions list will go, and where a message will go if there are no matching notes. There were two issues with this structure: - it was difficult to type because of how its structure overlapped a `NoteEntity` but didn't have a key field for discriminating the type. everything works just fine but the type signatures become harder to write. - we were storing display information in the `data` property and that, as a render concern, wasn't necessary Further, we were directly mutating `notes` which came from `filterNotes()` and the Redux state. We want to avoid directly mutating values we receive from other places in order to keep data flows clear and obvious. In this patch we are replacing the object data type for placeholders with simpler string counterparts, moving the label text into the `render` function, and exchanging a mutating operation for a non-mutating one. This is part of broader work to separate modifying the search parameters from actually filtering the notes and was originally created as part of an exploratory PR #1807.

Keeping up with React standards. This patch replaces the use of a ref function with a `createRef()`. This will end up making our types easier. This is part of broader work to separate modifying the search parameters from actually filtering the notes and was originally created as part of an exploratory PR #1807.

…utualized Follows exploratory work in #1807 For a long time we have had our own custom note list height cache. This was used to tell `react-virtualized` how tall each item in the list is so that it can determine how many and which ones to render. That cache was complicated and sensitive due to the caching and performance-oriented nature of the code. Whether `CellMesurerCache` existed when we first put in `react-virtualized` I don't know but it does do what we want it to do and with less guessing. It keeps track of the heights for the rendered components and provides a clean API to interface with the `AutoSizer`. In this patch we're removing our custom height cache and instead relying on the one provided by `react-virtualized`.

…tualized Follows exploratory work in #1807 For a long time we have had our own custom note list height cache. This was used to tell `react-virtualized` how tall each item in the list is so that it can determine how many and which ones to render. That cache was complicated and sensitive due to the caching and performance-oriented nature of the code. Whether `CellMesurerCache` existed when we first put in `react-virtualized` I don't know but it does do what we want it to do and with less guessing. It keeps track of the heights for the rendered components and provides a clean API to interface with the `AutoSizer`. In this patch we're removing our custom height cache and instead relying on the one provided by `react-virtualized`.

…tualized (#1829) Follows exploratory work in #1807 For a long time we have had our own custom note list height cache. This was used to tell `react-virtualized` how tall each item in the list is so that it can determine how many and which ones to render. That cache was complicated and sensitive due to the caching and performance-oriented nature of the code. Whether `CellMesurerCache` existed when we first put in `react-virtualized` I don't know but it does do what we want it to do and with less guessing. It keeps track of the heights for the rendered components and provides a clean API to interface with the `AutoSizer`. In this patch we're removing our custom height cache and instead relying on the one provided by `react-virtualized`. * special case the No Notes section so it retains its height Co-authored-by: Kat Hagan <kat@codebykat.com>

dmsnell mentioned this pull request Jan 4, 2020

Refactor(NoteList): Use already assigned list iterator #1808

Merged

dmsnell mentioned this pull request Jan 4, 2020

Refactor(NoteList): Use numbers for tabIndex instead of strings #1809

Merged

dmsnell mentioned this pull request Jan 4, 2020

Refactor(NoteList): Use createRef() instead of ref function #1811

Merged

dmsnell mentioned this pull request Jan 4, 2020

Refactor(NoteList): Reshape composite note list data structure #1810

Merged

dmsnell mentioned this pull request Jan 10, 2020

Note List: Replace custom height cache with one provided by react-virtualized #1829

Merged

Add types to the NoteList

86b89ac

dmsnell force-pushed the types/type-note-list branch from 4c34883 to 86b89ac Compare January 10, 2020 21:06

dmsnell closed this Jan 28, 2020

dmsnell deleted the types/type-note-list branch January 28, 2020 15:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Types/type note list #1807

WIP: Types/type note list #1807

dmsnell commented Jan 4, 2020

WIP: Types/type note list #1807

WIP: Types/type note list #1807

Conversation

dmsnell commented Jan 4, 2020