-
-
Notifications
You must be signed in to change notification settings - Fork 120
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
feat: carousel integration with history navigation #1601
Closed
aegroto
wants to merge
78
commits into
stackernews:master
from
aegroto:enhancement-backward-carousel-images
+2,265
−1,119
Closed
Changes from 2 commits
Commits
Show all changes
78 commits
Select commit
Hold shift + click to select a range
8745e4a
feat: carousel integration with history navigation
aegroto 4239f9a
Merge branch 'master' of github.com:stackernews/stacker.news into enh…
aegroto ec2ab58
Merge branch 'master' into enhancement-backward-carousel-images
huumn c691259
Merge branch 'master' of github.com:stackernews/stacker.news into enh…
aegroto a7a16e4
Merge branch 'enhancement-backward-carousel-images' of github.com:aeg…
aegroto 3be1418
fix: navigation-based carousel close action
aegroto 3d2063a
Use invoice.cancelledAt to determine if invoice expired or was cancel…
ekzyis 7d0eb60
add in error message how high are estimated fees when too high (#1634)
riccardobl b8dd4c4
fix LNC attachment and docs (#1639)
riccardobl 6b44b14
Remove unused function usePayment (#1641)
ekzyis 0039700
refactor: Check darkmode in useWallets (#1640)
ekzyis 4aba93f
Only fetch logs when we need them (#1638)
ekzyis ad4644f
Create useWalletStatus and useWalletSupport (#1646)
ekzyis 2bed712
Fix inconsistent actionArgs on retry (#1651)
ekzyis 971b510
direct receives and send paid action (#1650)
huumn 57a30a4
Wallet filters (#1627)
ekzyis 5c3ca67
use flexbox for wallet card header and make logos more consistent (#1…
riccardobl b7fd9e2
Improve LNC realiability by being nicer (#1658)
riccardobl e30185e
fix: cannot add images above text (#1659)
Soxasora 3a7e1d9
Remove unused useWallet from QR code component (#1660)
ekzyis 6f1dabb
Fix deposit push notifications (#1662)
ekzyis acdfdb2
Fix comment
ekzyis d42dc91
sender fallbacks
ekzyis 2b20cfa
Fix SenderError name
ekzyis 4b39e98
Fix TypeError
ekzyis 8976b30
Fix old invoice passed to QR code
ekzyis 0911ec3
Remove unnecessary sort
ekzyis b860a57
Fix payments if recv-only wallet enabled
ekzyis 1ddc4fa
Refactor wallet error handling with inheritance
ekzyis c49b6f8
Abort payment on unexpected errors
ekzyis 6b4dbed
Add comment when err.newInvoice is not set
ekzyis b562412
Ignore wallet configuration errors in QR code
ekzyis 2f1833c
Fix last wallet not returning new invoice
ekzyis 2bd928d
Allow retries of pessimistic actions
ekzyis f011bbc
Fix payment method returned by retries
ekzyis 748cc22
Show aggregated wallet errors in QR code
ekzyis 250b64b
Return last attempted invoice in canceled state
ekzyis 16a28c9
Fix filter for wallets that can send
ekzyis 2b64fef
Fix invoice retry even if no payment was attempted
ekzyis 85ece35
Fix missing item invoice update for optimistic actions
ekzyis f4426bb
Remove unnecessary error handling in LNC
ekzyis 744602f
Return latest state of paid or failed invoice
ekzyis e24e118
Create wrapped invoices on p2p zap retries
ekzyis b1f064d
Only retry same receiver if forward did not fail
ekzyis f2e1806
refactor out array of hooks
huumn ded2096
Fix wallet save
ekzyis de598c1
Fix [undefined] in logs
ekzyis bb9fc60
readability improvements
huumn 9574cd2
Fix missing item invoice update on failure
ekzyis fda130a
make logger use full wallet
huumn 4f05a9e
usesendwallets
huumn 9ea9352
fix zap fallback retries in notifications
huumn 0abbc6b
make fallback retry cache updates a special case
huumn 0774b67
function for merging data after retry
huumn f5ea386
fix cache update option name on qr
huumn 91aa1f6
feat: recent unpaid bounties selection (#1589)
aegroto fc9b8f0
Fix Territories selector updates without hard-reload (#1619)
aegroto d63f466
Introduce SubPopover (#1620)
felipebueno d890ab7
Fix missing authentication check for invite revocation (#1666)
ekzyis 7ef6fd2
Fix: progress bar shown on back navigation through pathname check (#1…
aegroto aff5f8d
fix: top boosts shows others' unpaid boosts (#1647)
Soxasora 6c86b39
fix can't upload from iOS camera/mov files (#1667)
Soxasora 6abe6ea
Custom invite code and note (#1649)
riccardobl 42da56e
stop probable source of 504 toasts
huumn 107aefd
Dockerfile update
aegroto 977d4dd
fix: fixed dockerfile
aegroto e50caa2
chore: removed commented dockerfile
aegroto edd83a1
feat: bundle action scaffolding
aegroto c953638
fix: added environment set-up on bundle action
aegroto 42a25e0
chore: printing environment in action
aegroto a74ea83
fix: changed base CI image and fixed environment creation
aegroto 8f0db77
chore: restore base ubuntu image
aegroto c7b4054
fix: test VAPID keys with the right bytes count
aegroto 7eaed29
fix: using actually generated VAPID keys
aegroto f5a67ac
chore: temporarily disabled compare steps
aegroto de3562f
Revert "chore: temporarily disabled compare steps"
aegroto 999846b
Fix edit timer stuck at 00:00 (#1673)
ekzyis e2d5380
item referral threshold
huumn File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't a shallow push. On backwards navigation, this closes the carousel by reloading the page. We do not want to reload the page, we only want to close the carousel on backwards navigation.Mhh, on closer inspection, maybe this doesn't trigger a page reload on backward navigation so this could be fine. However, I am still curious why you couldn't use
next/router
.Why not?
You also need to pop the state again on close since else, manually closing an image will mean that your next backward navigation will do nothing except popping that stale state.
So if I open many images and close all of them myself, I need to click back as many times as I opened images before I actually go back.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not been able to use
router.push
because it was not updating the history anyway. I don't know the reason, but I believe it may be related to the path being identical to the last one, and I guess the router drops duplicate entries. An alternative would be to use a different URL, but this hinders the modal to be opened, I still have to investigate this behaviour.I didn't understand that the push had to be unique for the entire carousel. I will work on this as soon as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a query param might work:
router.push({ pathname: router.pathname, query: { carousel: true, ...router.query}}, router.asPath, { shallow: true }}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what I've been trying to do but it looks like the carousel is immediately closed after a 'router.push' or 'router.replace' call. This does not happen when calling native `window' APIs though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you used
shallow: true
and it did that, then I'd guess something is rerendering that shouldn't.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is due to line 73 of
modal.js
:The push call triggers this event, which then calls maybeOnClose. This makes sense as the native window API is not trigger router's events.