Remove share providers and fix various share bugs #18
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- The abstraction was making this code more complicated - Also fix a few url encoding issues
|
cdrini
force-pushed
the
refactor/share-providers
branch
2 times, most recently
from
59db1d3 to
cc5b621
Compare
cdrini
force-pushed
the
refactor/share-providers
branch
from
cc5b621 to
6d47a91
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #18 +/- ##
==========================================
- Coverage 94.53% 94.51% -0.02%
==========================================
Files 15 9 -6
Lines 1664 1513 -151
Branches 61 52 -9
==========================================
- Hits 1573 1430 -143
+ Misses 88 81 -7
+ Partials 3 2 -1 ☔ View full report in Codecov by Sentry. |
iisa
approved these changes
Feb 15, 2024
| 'Free Download, Borrow, and Streaming', | ||
| 'Internet Archive', | ||
| ] | ||
| .filter(Boolean) |
There was a problem hiding this comment.
q: why do we need to filter? should we have defaults for the desc, creator?
There was a problem hiding this comment.
Sometimes things didn't have a creator was the main case.
| it('toggles visibility of embed options', async () => { | ||
| const el = (await fixture(container())) as IauxSharingOptions; | ||
|
|
||
| el.toggleEmbedOptions(new Event('click')); |
There was a problem hiding this comment.
we can probably do...
Suggested change
| el.toggleEmbedOptions(new Event('click')); | |
| el.shadowRoot.querySelector('embed dropdown').click(); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I started by trying to find a way to specify a direct share url from bookreader, but couldn't quite find a way to make that happen in a reasonable fashion. But did find some nice cleanups to do while investigating the code!
URLSearchParamseverywhere; fixed various url encoding issues