#5531 toggle issue fix #5562

prachi00 · 2023-04-08T00:07:25Z

Thank you for your contribution to the KodaDot NFT gallery.

👇 _ Let's make a quick check before the contribution.

PR Type

Bugfix
Feature
Refactoring

Context

Closes "do nothing" when clicking icon again #5531
Requires deployment <>

Before submitting pull request, please make sure:

My contribution builds clean without any errors or warnings
I've merged recent default branch -- main and I've no conflicts
I've tried to respect high code quality standards
I've didn't break any original functionality
I've posted a screenshot of demonstrated change in this PR

Optional

I've tested it at </bsx/collection>
I've tested PR on mobile
I've written unit tests 🧪
I've found edge cases

Had issue bounty label?

Fill up your KSM address:
Payout

Community participation

Are you at KodaDot Discord?

Screenshot 📸

My fix has changed something on UI; a screenshot is best to understand changes for others.

netlify · 2023-04-08T00:07:37Z

✅ Deploy Preview for koda-canary ready!

Name	Link
🔨 Latest commit	`62df37e`
🔍 Latest deploy log	https://app.netlify.com/sites/koda-canary/deploys/64377008bbea410008dc2414
😎 Deploy Preview	https://deploy-preview-5562--koda-canary.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

reviewpad · 2023-04-08T00:07:42Z

AI-Generated Summary: This pull request addresses issue #5531 by simplifying the handling of the wallet connect modal in the ProfileDropdown.vue and ConnectWalletButton.vue components. It reduces code duplication and emits a toggleConnectModal event instead of directly managing the modal within the ConnectWalletButton.vue.

reviewpad · 2023-04-08T00:11:49Z

AI-Generated Summary: This pull request consists of two patches:

Fixes the toggle issue ("do nothing" when clicking icon again #5531) by updating the event handler's name from @closeBurgerMenu to @toggleConnectModal in ProfileDropdown.vue. It also simplifies the toggleWalletConnectModal method in ConnectWalletButton.vue by removing the unnecessary code related to modal.
Removes a useless import (ConnectWalletModalConfig) from ConnectWalletButton.vue file, which further optimizes the code.

Jarsen136 · 2023-04-08T02:43:10Z

components/shared/ConnectWalletButton.vue

-      ...ConnectWalletModalConfig,
-    })
-
-    this.$emit('closeBurgerMenu')


pls do not delete this stuff. The changes will break the functionality to open the modal on mobile.

oh, can we add mobile check then? we dont need this on web, its what was breaking this issue

Is there any better way to handle this case? The function name is toggleWalletConnectModal which is not related to mobile check I guess : )

reviewpad · 2023-04-08T23:27:54Z

AI-Generated Summary: This pull request consists of 3 patches that primarily fix a toggle issue:

The first patch addresses the toggle issue by updating the event listener in ProfileDropdown.vue and removing unnecessary lines of code in ConnectWalletButton.vue.
The second patch removes a now-unused import (ConnectWalletModalConfig) from ConnectWalletButton.vue.
The third patch adds a mobile device check in ConnectWalletButton.vue, bringing back the removed ConnectWalletModalConfig import and utilizing the isMobileDevice utility function to conditionally handle the toggle behavior based on whether the user is on a mobile device or not.

components/navbar/ProfileDropdown.vue

daiagi

Connect button doesn't work on some device
I have a laptop that can be folded into tablet and it doesn't work on this device (when folded in tablet mode)
probably because of this:
import { isMobileDevice } from '@/utils/extension'

please don't use this to check screen dimensions, it doesn't work as expected, it checks user agent...

reviewpad · 2023-04-10T01:12:16Z

AI-Generated Summary: This pull request contains 4 commits that address the issue with toggling the wallet connect modal on mobile devices, as well as some refactoring and code clean-up. Specifically, the changes include:

Updating the event listener in ProfileDropdown.vue to use toggleWalletConnectModal instead of closeBurgerMenu.
Removing an unused import in ConnectWalletButton.vue.
Adding a mobile device check in ConnectWalletButton.vue to conditionally open/close the wallet connect modal.
Refactoring code in ProfileDropdown.vue and ConnectWalletButton.vue to improve readability and maintainability.

prachi00 · 2023-04-10T01:13:24Z

Connect button doesn't work on some device I have a laptop that can be folded into tablet and it doesn't work on this device (when folded in tablet mode) probably because of this: import { isMobileDevice } from '@/utils/extension'

please don't use this to check screen dimensions, it doesn't work as expected, it checks user agent...

maybe in another issue we can refactors this utils function then if it isnt usable, for now I have used
private isMobile = ref(window.innerWidth < 1024)

which is the same check we are using to show the menu on mobile so all good

daiagi · 2023-04-10T08:14:32Z

wfm on desktop
could you do the same for tablet?

screen-capture.webm

daiagi · 2023-04-10T08:19:24Z

for now I have used
private isMobile = ref(window.innerWidth < 1024)

works now. thanks

reviewpad · 2023-04-10T19:38:04Z

AI-Generated Summary: This pull request includes four patches that address the following changes:

Fixing the toggle issue ("do nothing" when clicking icon again #5531) by updating the event listeners in the ProfileDropdown.vue and ConnectWalletButton.vue components.
Removing an unused import statement in ConnectWalletButton.vue.
Adding a mobile check using isMobileDevice function and conditionally update the modal handling in the toggleWalletConnectModal method in ConnectWalletButton.vue.
Performing minor refactors for code readability in ProfileDropdown.vue and ConnectWalletButton.vue components.

reviewpad · 2023-04-10T21:33:13Z

AI-Generated Summary: This pull request contains a series of patches that fix the toggle issue #5531, remove a useless import, add a mobile check, and refactor some code. The patches modify the ProfileDropdown.vue and ConnectWalletButton.vue files, adjusting their functionalities, removing unnecessary imports, and adding a mobile check to better handle wallet connection. The refactors include minor changes to enhance readability and maintainability of the code.

Jarsen136

It's a strange fix but works for me

prachi00 · 2023-04-13T02:58:41Z

wfm on desktop could you do the same for tablet?

bit tricky to do the same on mobile, pulling this in for a separate issue

codeclimate · 2023-04-13T02:59:46Z

Code Climate has analyzed commit 62df37e and detected 0 issues on this pull request.

View more on Code Climate.

reviewpad · 2023-04-13T02:59:59Z

AI-Generated Summary: This pull request includes 4 different patches:

Fixes the toggle issue in the ProfileDropdown component by changing the event listener from @closeBurgerMenu to @toggleConnectModal.
Removes an unnecessary import of ConnectWalletModalConfig from ConnectWalletButton.vue.
Adds a mobile device check using isMobileDevice utility function and updates the toggleWalletConnectModal method in the ConnectWalletButton.vue to handle different behaviors on mobile and desktop devices.
Refactors code for closing the burger menu in ProfileDropdown.vue and updates the ConnectWalletButton.vue component to make use of a ref, isMobile, for checking the device type.

daiagi

bug fixed on desktop
needs fixing in mobile /tablet as well

Tablet double modal opening up issue #5636

yangwao · 2023-04-14T18:35:35Z

pay 30 usd

yangwao · 2023-04-14T18:35:47Z

😍 Perfect, I’ve sent the payout
💵 $30 @ 37.26 USD/KSM ~ 0.805 $KSM
🧗 EzGc4s9PgCPx1YnF3fqzhLzVHpHMTL4LWPScwpDrR8JKgSU
🔗 0x6aacb845986c9709697b23daa0ca30bc0bd3b4744701cff8c243fbc4d055c20a

🪅 Let’s grab another issue and get rewarded!
🪄 github.com/kodadot/nft-gallery/issues

#5531 toggle issue fix

3a7a458

prachi00 requested a review from a team as a code owner April 8, 2023 00:07

prachi00 requested review from daiagi and Jarsen136 and removed request for a team April 8, 2023 00:07

reviewpad bot added small Pull request is small waiting-for-review labels Apr 8, 2023

remove useless import

2fe51a0

Jarsen136 requested changes Apr 8, 2023

View reviewed changes

exezbcz added the S-visual-ok-✅ label Apr 8, 2023

add mobile check

a5ffce5

prachi00 requested a review from Jarsen136 April 8, 2023 23:27

Jarsen136 reviewed Apr 9, 2023

View reviewed changes

components/navbar/ProfileDropdown.vue Show resolved Hide resolved

Jarsen136 added the S-changes-requested-🤞 PR is almost good to go, just some fine tunning label Apr 9, 2023

daiagi requested changes Apr 9, 2023

View reviewed changes

refactors

87e995e

prachi00 requested review from daiagi and Jarsen136 April 10, 2023 01:13

Merge branch 'main' into feat-toggle-fix

646e20c

Merge branch 'main' into feat-toggle-fix

ea3da78

Jarsen136 approved these changes Apr 12, 2023

View reviewed changes

Jarsen136 added S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked and removed S-changes-requested-🤞 PR is almost good to go, just some fine tunning labels Apr 12, 2023

prachi00 mentioned this pull request Apr 13, 2023

Tablet double modal opening up issue #5636

Closed

Merge branch 'main' into feat-toggle-fix

62df37e

daiagi approved these changes Apr 13, 2023

View reviewed changes

yangwao merged commit 8aac5f5 into main Apr 14, 2023
17 checks passed

yangwao deleted the feat-toggle-fix branch April 14, 2023 18:35

yangwao added the paid pull-request has been paid label Apr 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#5531 toggle issue fix #5562

#5531 toggle issue fix #5562

prachi00 commented Apr 8, 2023 •

edited

netlify bot commented Apr 8, 2023 •

edited

reviewpad bot commented Apr 8, 2023

reviewpad bot commented Apr 8, 2023

Jarsen136 Apr 8, 2023

prachi00 Apr 8, 2023

Jarsen136 Apr 9, 2023

reviewpad bot commented Apr 8, 2023

daiagi left a comment •

edited

reviewpad bot commented Apr 10, 2023

prachi00 commented Apr 10, 2023

daiagi commented Apr 10, 2023

daiagi commented Apr 10, 2023

reviewpad bot commented Apr 10, 2023

reviewpad bot commented Apr 10, 2023

Jarsen136 left a comment

prachi00 commented Apr 13, 2023

codeclimate bot commented Apr 13, 2023

reviewpad bot commented Apr 13, 2023

daiagi left a comment

yangwao commented Apr 14, 2023

yangwao commented Apr 14, 2023

#5531 toggle issue fix #5562

#5531 toggle issue fix #5562

Conversation

prachi00 commented Apr 8, 2023 • edited

PR Type

Context

Before submitting pull request, please make sure:

Optional

Had issue bounty label?

Community participation

Screenshot 📸

netlify bot commented Apr 8, 2023 • edited

✅ Deploy Preview for koda-canary ready!

reviewpad bot commented Apr 8, 2023

reviewpad bot commented Apr 8, 2023

Jarsen136 Apr 8, 2023

Choose a reason for hiding this comment

prachi00 Apr 8, 2023

Choose a reason for hiding this comment

Jarsen136 Apr 9, 2023

Choose a reason for hiding this comment

reviewpad bot commented Apr 8, 2023

daiagi left a comment • edited

Choose a reason for hiding this comment

reviewpad bot commented Apr 10, 2023

prachi00 commented Apr 10, 2023

daiagi commented Apr 10, 2023

daiagi commented Apr 10, 2023

reviewpad bot commented Apr 10, 2023

reviewpad bot commented Apr 10, 2023

Jarsen136 left a comment

Choose a reason for hiding this comment

prachi00 commented Apr 13, 2023

codeclimate bot commented Apr 13, 2023

reviewpad bot commented Apr 13, 2023

daiagi left a comment

Choose a reason for hiding this comment

yangwao commented Apr 14, 2023

yangwao commented Apr 14, 2023

prachi00 commented Apr 8, 2023 •

edited

netlify bot commented Apr 8, 2023 •

edited

daiagi left a comment •

edited