- Notifications
You must be signed in to change notification settings - Fork7
Comparing changes
Open a pull request
base repository:github/quote-selection
Uh oh!
There was an error while loading.Please reload this page.
base:main
head repository:github/quote-selection
Uh oh!
There was an error while loading.Please reload this page.
compare:keithate-deprecate_shortcuts
Uh oh!
There was an error while loading.Please reload this page.
- 18commits
- 3files changed
- 2contributors
Commits on Nov 2, 2021
introduce
getSelection(): SelectionContext
This DRYs up functionality, pulling some of the complexity out of othermethods.Co-authored-by: Kate Higa <khiga8@github.com>
Got rid of quoteShortcut and replaced with quote
This simplifies tests: testing less for side-effects.Co-authored-by: Kate Higa <khiga8@github.com>
Remove keyboard event listener,
quoteSelection
method.BREAKING CHANGEThe code for handling eventlistening is very trivial, and might vary perconsumer of this library. It is not core functionality, and should beleft to upstream to setup.Removing this reduces the API surface area and complexity from this libraryas it doesn't need to concern itself with event management and keyboardshortcuts.Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>
BREAKING CHANGEThis was never used upstream, so we want to remove this in the interestof simplicity.Co-authored-by: Kate Higa <khiga8@github.com>
Removed signal and AbortController
Without any event listeners (onCopy and keydown have been removedprior) AbortController is effectively dead code and can be removed.Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>
Now that all functions can be used independently of `install()`, andoptions can be passed into those methods, it makes little sense to havethem in `install()`, and so it can be simplified to just providing thefunctionality of collecting containers.Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>
Remove
install()
, convertcontainer
tocontainerSelector
BREAKING CHANGE`install(container)` was side-effectful behavior which determined wherequote containers would be. A better design for this is to have functionsrequire a `containerSelector` to query against. This does the same thingbut is slightly more idiomatic.Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>
keithamus committedNov 2, 2021 This boolean was useful for onCopy but is no longer useful.Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>
This was only used to intercept quote behaviour, and doesn't really addany value as whatever used to listen to this can simply do the samebehaviour after `quote()` has been called.Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>
Remove quote method, export extractQuote/insertQuote
`quote` really does very little, it effectively calls `extractQuote()`followed by `insertQuote()`. Both can be exported functions and couldbe used instead, providing more flexibility.Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>
keithamus committedNov 2, 2021 Remove
getSelectionContext()
argument, addquoteElement
.We can remove `getSelectionContext` entirely, by inlining it into the`extractQuote` body, however it was useful to pass in a custom selectioncontext for the use case of selecting the contents of an element.By introducing `quoteElement` as an option we can supply the single usecase for creating custom `SelectionContext`s, which also can replacesome code we have upstream.
keithamus committedNov 2, 2021 Changed extractQuote to require
containerSelector
.BREAKING CHANGE`extractQuote` would be guaranteed to return `undefined` if`containerSelector` was not provided as an option, which means it is notoptional, and so should be an argument proper, as opposed to somethingin an options bag. This changes extractQuote to do just that.Co-authored-by: Kate Higa <khiga8@github.com>
Rename
focusNode
tostartContainer
The initial code created the `focusNode` const which was a differentname to the property it was retrieving: `startContainer`. Thissimplifies the code by making it more readable: less inventive namesprovides clearer intent as to the data manipulation that is happening.Co-authored-by: Kate Higa <khiga8@github.com>
Split
extractQuote
intoextractQuote
andasMarkdown
.The `quoteMarkdown` option of `extractQuote` was a "mode" -significantly changing the behaviour of the `extractQuote` function. Assuch it is better to split it out into a separate function which can beinvoked by client code - more explicit than setting an option.Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>
Drop event dispatch for callback in
asMarkdown
This drastically simplifies the `asMarkdown` function, avoiding theawkward try/catch and throwing on a setTimeout. The callback is also afar more idiomatic pattern for intercepting objects for mutation.Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>
BREAKING CHANGEThis function only ever used the `quoteElement` option, so it issimpler to have this as an optional argument as opposed to having anoptions object passed in.Co-authored-by: Kate Higa <khiga8@github.com>
Commits on Nov 3, 2021
Commits on Nov 4, 2021
This comparison is taking too long to generate.
Unfortunately it looks like we can’t render this comparison for you right now. It might be too big, or there might be something weird with your repository.
You can try running this command locally to see the comparison on your machine:git diff main...keithate-deprecate_shortcuts
Uh oh!
There was an error while loading.Please reload this page.