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

Merge release 1.14 into master #1863

Merged
merged 61 commits into from
Jan 27, 2020
Merged

Merge release 1.14 into master #1863

merged 61 commits into from
Jan 27, 2020

Conversation

dmsnell
Copy link
Member

@dmsnell dmsnell commented Jan 24, 2020

Merge changes in release/1.14 into master.

codebykat and others added 30 commits November 21, 2019 12:07
This PR adds matching tag results to the notes list.

It shows a maximum of five tags and hides the section if there are no matches. Clicking a suggested adds a query to the search field with the "tag:" syntax.
Updates dependencies to the latest.
We were directing users to /settings?source=blah which GAE automatically
redirected to /settings/ removing the url param. Additionally, the code always
returned 'electron' as the source which is not only incorrect but GAE only
understands the source as react. From the web app and from electron we should
send the source as react so that we hide settings that only apply to the old
web app.
…on (#1741)

There was a div that was draggable underneath the revision selector.
Additionally, the revision selector was draggable. In remedying this we are no
longer rendering the note toolbar header while the revision panel is open. We
are also not rendering the revision panel when it is not open. The showing and
hiding of these panels is overly complex and should be looked at in a future
PR.
* Update Dependencies

* Update release notes
Added "GenericName" field to the desktop option for Linux (so that its icon displays a useful subtitle when hovered over in KDE panel).

Since the convention is a short descriptive name of what the app is (e.g. Firefox uses "Web Browser", GIMP uses "Image Editor", Atom uses "Text Editor"), I used "Note Taking Application".
)

At some point, some code was added that allowed the tabs to scroll. In adding
this it caused the settings tab to display at full height and not be contained
by the scroll box in the modal. This fixes
https://href.li/?https://github.com/Automattic/simplenote-electron/issues/1268
The inline button component to copy the publish url was lacking an indication
that the text was copied. Additionally, the current setup was using a copy
method that can have an issue in some browsers. This commit switches to use of the
clipboard npm repo, moves code into a component, and adds an indication that
the text was copied.
We are no longer using renovate. This removes the config.
The prettier command was not properly checking all files under code revision.
It was only checking files that were one layer deep. This updates the command
so that it runs on all files and applies formatting to those files
The email field for collaborators allowed for incomplete emails. When entering
an incomplete email it would add that partial as a tag on the note. This fix
also disallows the user from adding themseleves as a collaborator.
This is a good small example of how React hooks are used.  Nothing changes
except we are now using a functional component and using hooks over the lifecycle
methods.
* Lint Fixes

* Fix more lint issues
The replication bug needed this util to prevent it from happening. Thankfully
we were able to resolve it via another fix. This tab restriction is no longer
needed.
* Add elint plugin for React Hooks

* Update release notes
After restoring a note the editor was stuck o the selected revision. This PR
clears that revision from the flux state. Additionally, that restoration of a
note was not causing a sync to occur.  This triggers a note sync that updates
everything and cements the restored version as the current version.
* Add git hooks to run formats, lints, and tests

* Add release notes
* Fixes shortcut to enable Markdown preview

Ctrl/Cmd + Shift + p is supposed to enable the markdown preview. There were two
errors. We were checking for a capital 'P' when we should have been checking
for 'shift' and lowercase 'p'. We were also using a bad check for markdown.

* Add release note

* ensure lowercase to ensure the logic is correct
* Stop running lint and tests on pre-push git hook

These tests already run in CI after pushing changes and before pushing
they add considerable delay. Many of the issues the tests reveal aren't
completely relevant for work-in-progress branches.

* Add lint tests to CI build
* Add TypeScript support and demo small refactor

As if the types and data flows weren't complicated enough in this
application, we have no support from the language or compiler to make
sense of it all.

In this patch we're adding TypeScript into the build pipeline to allow
for writing modules in TypeScript and to aid us in knowing just what we
are dealing with.

Types are stripped from the modules before sending to Babel and so
therefore we won't prevent a build just because there are type errors.

This patch enables a certain level of autocomplete on `appState`,
indicating which action creators are available.

* Exclude TypeScript files from no-unused-var

There's an issue with false positives for type imports.

typescript-eslint/typescript-eslint#363

* Do a little more fanangling on types
dmsnell and others added 25 commits December 31, 2019 00:20
When creating a new note we know that the operation might fail but we
keep on treating it as a success.

In this patch we send a debug message if the note bucket fails to create
our note and we don't continue processing as if it had.
* add listener in browser shell to respond to change of system theme

* release notes

* mock window.matchMedia which is not defined in JSDom
* Update Dependencies
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.
* TypeScript: Rename all files in `lib/` as `.ts`/`.tsx`

We've been carving out individual files in separate PRs but haven't seen any
issues with converting to TypeScript files. In this patch we're doing it in
bulk so we can stop conflating changes with renames and so that we'll benefit
from the additional type-checking going on throughout the app.

* Use tsx for test files that conatin jsx

* Update snapshots

Co-authored-by: Jonathan Belcher <[email protected]>
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.
In #1792 I introduced a typo from `name` to `namer` and this prevented the tag
drawer from working properly. That is, when trying to filter by tag it was
looking for matches on `tag.data.namer` instead of on `tag.data.name` and
unsuprisingly there were no matches.

After this patch things are restored to normal working order.
When showing each item in the note list we run the content decorators on them
in order to show checkboxes instead of the markdown syntax for a checkbox and
we also highlight the matching text from the search box.

Previously we have only been showing the first match for each decorator because
of a couple small bugs:
 - the task prefix matcher wasn't using the `m` multi-line flag in its RegExp
   pattern
 - the search filter text was only replacing the first occurrance, see JS
   "replaceAll()" proposal

In this patch we're rewritten `decorateWith` to automatically create a global
RegExp pattern for the search text and the task prefix RegExp pattern has been
corrected.

After this patch you can now see all of the decorated items in the note list;
for example, if you had two checklist items in a note then you can now see
multiple checkboxes in the note list whereas before you'd only see the first
one and then the second one would appear as ` - [ ]` or `- [x]`
When the app loads there is potential for the is-note-open css class to be
missing on app-layout. This removes the reliance on isNoteOpen as it is note
needed and should be independent. Now focus mode class will remain on all
screen widths but will only change the layout for widths greater than 750px. We
should look at why is-note-open is not applied to app layout and how the layout
is set up in general. Fixed: #1525
Fixes: #1732 because of draft-js update

Updates Dependencies
Fixes: #676

The numbering of ol lists is currently broken when you have a list that is
interrupted with a line of text that is not part of the list.

This PR whitelists the start attribute on ol tags which fixes the issue.

For example:

1. One
1. Two
1. Three
some_text some_text
1. Four
The above example should number the list 1 to 4. It, however, orders the list 1
to 3 and then after the interrupt starts over at 1.

The output from showdown, the markdown to html processor, is:

<ol>
<li>One</li>
<li>Two</li>
<li>Three</li>
</ol>
<p>some_text some_text</p>
<ol start="4">
<li>Four</li>
</ol>
Notice the second ol has a start attribute <ol start="4">.

When we pass the html into the sanitizeHtml function that attribute is
stripped. This PR whitelists that attribute so it is not stripped.
Fixes: #676

When a list has a trailing `-` the markdown renderer (showdown) will make the
previous list element an h2. Technically this is valid markdown and is doing
what it should but this is usually not the intention of the user. Because we
auto-add  a `-` after a carriage return users run into this issue regularly.

This issue is resolved by enabling the `smoothLivePreview` option in showdown.

You can replicate this with the following markdown:

```
- test 1
- test 2
- test 3
-
```
Currently, this will output:

```
<ul>
<li>test 1</li>
<li>test 2</li>
</ul>
<h2 id="test3">- test 3</h2>
```
After the addition of the `smoothLivePreview` option it will return:
```
<ul>
<li>test 1</li>
<li>test 2</li>
<li>test 3</li>
-
</ul>
```
Babel was outputting warnings about lodash and react-dom that it wasn't
removing white space. This sets babel compact to false in order to stop those
warnings. We also don't need babel to compact in development.
https://babeljs.io/docs/en/options#compact
The `filterNotes()` function is relatively expensive, particularly for accounts
with many notes.  Previously we have been running this several times in a
normal app update because it gets called in `mapStateToProps` on `note-list`
and also in a couple of other components. When these all re-render we end up
filtering the notes multiple times in a row even though no changes could have
occurred during that window of time (due to the single-threaded nature of
JavaScript, not because of the unlikelihood of receiving remote changes).

In this patch we're separating out the filtered notes into a new place in
`state` and we're updating it via middleware that watches for any actions which
might impact the note list. This gives us more control over the update cycle
and lets us replace debounced logic with synchronous updates.

After this change the search field's input will immediately update to reflect
the changes being entered in on the keyboard or via the tag list panel but the
note list won't immediately update when only typing in the search field. It
will update immediately when changing tags or when performing "atomic" updates
like trashing or restoring a note. The decision to defer or operate
"immediately" was based on the expectations of how interactive each action
should be. When we're typing in a search field we are probably going to type
more before we need the results to update, but when we flip to another tag we
expect that to immediately reflect visually.
* make tag match inclusive of unicode characters

* release notes
* Allow width attribute on img tags

* Update release notes
The builds on AppVeyor have been failing.  This should resolve the issue.

See this PR for more detail: #1842
In paFIJd-8t it was reported that note corruption was occurring when viewing the history of one note when it gets updated remotely. Although this fix may or may not address that problem a code audit revealed that in #1792 I removed what appeared to be an unused function when it was in fact used by the component's `onClickOutside` wrapper.

The missing handler function closed out of the revision slider when clicking anywhere else. Without it, the thread handling the click crashed but since it was a callback it didn't crash the app. A message appeared in the developer console indicating that the handler was missing.

Because we haven't been closing the revision slider it has become possible to start editing a note while the revisions are still open, something prevented before by way of the click handler. Thus, if viewing a revision, clicking anywhere else, and continuing, then we fill the editor with what it thinks is a note (and not a revision) and then will respond to remote updates with the contents of that revision before accepting remote updates.

(cherry picked from commit 43bc2a5)
@dmsnell dmsnell requested a review from a team January 24, 2020 18:07
@dmsnell dmsnell merged commit 65561d6 into master Jan 27, 2020
@dmsnell dmsnell deleted the release/1.14 branch January 27, 2020 17:07
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.

6 participants