Skip to content

Commit

Permalink
Refactor(NoteList): Reshape composite note list data structure (#1810)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dmsnell authored Jan 6, 2020
1 parent eba2eb7 commit b3b1603
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 26 deletions.
2 changes: 1 addition & 1 deletion RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
- Added React Hooks ESLint Plugin [#1789](https://github.com/Automattic/simplenote-electron/pull/1789)
- Added end-to-end testing with Spectron [#1773](https://github.com/Automattic/simplenote-electron/pull/1773)
- Removed a workaround for indexing note pinned status [#1795](https://github.com/Automattic/simplenote-electron/pull/1795)
- Maintenance cleanups [#1796](https://github.com/Automattic/simplenote-electron/pull/1796), [#1797](https://github.com/Automattic/simplenote-electron/pull/1797), [#1808](https://github.com/Automattic/simplenote-electron/pull/1808), [#1809](https://github.com/Automattic/simplenote-electron/pull/1809)
- Maintenance cleanups [#1796](https://github.com/Automattic/simplenote-electron/pull/1796), [#1797](https://github.com/Automattic/simplenote-electron/pull/1797), [#1808](https://github.com/Automattic/simplenote-electron/pull/1808), [#1809](https://github.com/Automattic/simplenote-electron/pull/1809), [#1810](https://github.com/Automattic/simplenote-electron/pull/1810)
- Updated dependencies [#1802](https://github.com/Automattic/simplenote-electron/pull/1802)

## [v1.13.0]
Expand Down
41 changes: 16 additions & 25 deletions lib/note-list/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,12 @@ const rowHeightCache = f => (
const note = notes[index];

// handle special sections
switch (note.type) {
case 'header':
switch (note) {
case 'notes-header':
return HEADER_HEIGHT;
case 'tag-suggestions':
return HEADER_HEIGHT + TAG_ROW_HEIGHT * tagResultsFound;
case 'empty':
case 'no-notes':
return EMPTY_DIV_HEIGHT;
}

Expand Down Expand Up @@ -206,11 +206,11 @@ const renderNote = (
const note = notes['undefined' === typeof index ? rowIndex : index];

// handle special sections
switch (note.type) {
case 'header':
switch (note) {
case 'notes-header':
return (
<div key={key} style={style} className="note-list-header">
{note.data}
Notes
</div>
);
case 'tag-suggestions':
Expand All @@ -219,10 +219,10 @@ const renderNote = (
<TagSuggestions />
</div>
);
case 'empty':
case 'no-notes':
return (
<div key={key} style={style} className="note-list is-empty">
<span className="note-list-placeholder">{note.data}</span>
<span className="note-list-placeholder">No Notes</span>
</div>
);
}
Expand Down Expand Up @@ -285,24 +285,15 @@ const renderNote = (
* @returns {Object[]} modified notes list
*/
const createCompositeNoteList = (notes, filter, tagResultsFound) => {
if (filter.length > 0 && tagResultsFound > 0) {
if (notes.length === 0) {
notes.push({
type: 'empty',
data: 'No Notes',
});
}

notes.unshift({
type: 'header',
data: 'Notes',
});
notes.unshift({
type: 'tag-suggestions',
data: 'Tag Suggestions',
});
if (filter.length === 0 || tagResultsFound === 0) {
return notes;
}
return notes;

return [
'tag-suggestions',
'notes-header',
...(notes.length > 0 ? notes : ['no-notes']),
];
};

export class NoteList extends Component {
Expand Down

0 comments on commit b3b1603

Please sign in to comment.