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.

Sign up for GitHub

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

Jump to bottom

[BUMP SDK IN DAPP] - Fix the the modal close behaviour on click outside modal #1611

Merged
merged 5 commits into from
Jul 2, 2024
Merged

[BUMP SDK IN DAPP] - Fix the the modal close behaviour on click outside modal #1611

merged 5 commits into from
Jul 2, 2024

Conversation

abhishek-01k
Copy link
Collaborator

@abhishek-01k abhishek-01k commented Jun 7, 2024 •
edited
Loading

Pull Request Template

Description

Create Group Modal will not be closed when clicked outside.

  • Problem/Feature:

Type of Change

  • Bug fix
  • New feature

Checklist

  • Quick PR: Is this a quick PR? Can be approved before finishing a coffee.
    • Quick PR label added
  • Not Merge Ready: Is this PR dependent on some other PR/tasks and not ready to be merged right now.
    • DO NOT Merge PR label added

Frontend Guidelines

Build & Testing

  • No errors in the build terminal
  • Engineer has tested the changes on their local environment
  • Engineer has tested the changes on deploy preview

Screenshots/Video with Explanation

  • Before: Explain the previous behavior

  • Create Group Modal in the Chats is not persistent. Once you open the modal and click away on the dapp (i.e, toggle the light mode and dark mode) so the modal goes away.

  • After: What's changed now

  • Once you open the modal and click away on the dapp (i.e, toggle the light mode and dark mode) so the modal goes away.

Additional Context

Review & Approvals

  • Self-review completed
  • Code review by at least one other engineer
  • Documentation updates if applicable

Notes

CreateGroupModal from @pushprotocol/uiweb

  • It has a new prop closeModalOnClickAway which if passed true will close the modal
Screenshot 2024-06-24 at 10 54 39 AM

UserProfile from @pushprotocol/uiweb

  • It also has a new prop closeUserProfileModalOnClickAway which if passed true will close the Profile Modal on ClickAway.
Screenshot 2024-07-01 at 5 33 33 PM

ChatView from @pushprotocol/uiweb

  • It has a new prop closeChatProfileInfoModalOnClickAway which if passed true will close the Group Info Modal on ClickAway.
Screenshot 2024-07-01 at 5 34 04 PM

Things that you need to test

  • Check if the User Profile modal is getting closed or not in the clickaway
  • Check if the group info modal is getting closed or not in the clickway.
  • See if the modal is visible when their is a chat id and user reloads the dapp in the chat route.

@abhishek-01k abhishek-01k self-assigned this Jun 7, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 7, 2024

All looks good.

@abhishek-01k abhishek-01k linked an issue Jun 7, 2024 that may be closed by this pull request
🐛 [BUG] - [BUMP SDK IN DAPP] - Fix the the modal close behaviour on click outside modal #1561
Closed
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 7, 2024 •
edited
Loading

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-07-02 08:57 UTC

@mishramonalisha76
Copy link
Contributor

@abhishek-01k should we close the modal when disconnect the wallet in the dapp? because i think if we dont close it creating a group will give error.

@abhishek-01k abhishek-01k added the DO NOT MERGE If the PR/task is dependent on some other task add this label to the PR. label Jun 7, 2024
@abhishek-01k abhishek-01k added Quick PR A PR that can be approved before finishing a coffee and removed DO NOT MERGE If the PR/task is dependent on some other task add this label to the PR. labels Jun 7, 2024
mishramonalisha76
mishramonalisha76 suggested changes Jul 1, 2024
View reviewed changes
src/sections/chat/ChatSection.tsx
@@ -89,7 +89,7 @@ const ChatSection = ({ chatId, setChatId, loggedIn }) => {

{/* Render unlock profile here if user is not logged in and chat instance is loaded */}
{userPushSDKInstance && userPushSDKInstance?.readmode() && chatId && (
<UnlockProfileWrapper type={UNLOCK_PROFILE_TYPE.BOTTOM_BAR} />
<UnlockProfileWrapper type={UNLOCK_PROFILE_TYPE.MODAL} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i still see it in the bottom bar, what does this change mean

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, when you have chat Id in the url and then you reload when you have not saved PGP keys in storage. You will see a modal instead of Bottom Bar.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 1, 2024

All looks good.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 1, 2024

In the package.json file:

  1. In the "start" script, there is a typo in "node dev-mode-link-local-sdk.mjs prodsdk cleanup nostart && yarn dev". It should be "node dev-mode-link-local-sdk.mjs prodsdk cleanup nostart && vite dev".
  2. In the "build" script, there is a typo in "NODE_OPTIONS=--max-old-space-size=8192 node dev-mode-link-local-sdk.mjs prodsdk cleanup nostart && vite build". It should be "NODE_OPTIONS=--max-old-space-size=8192 node dev-mode-link-local-sdk.mjs prodsdk cleanup nostart && vite build".
  3. In the "change" script, the second command "node build.mjs dev" is missing the "&&" before it. It should be "node dev-mode-link-local-sdk.mjs prodsdk cleanup nostart && node build.mjs dev".

In the ChatSection.tsx file:

  1. There is an unclosed comment block after "<Welcome".
  2. The styled component "IntroContainer" is missing a closing curly brace "}" at the end.
  3. The styled component "BackContainer" is missing a closing curly brace "}" at the end as well.

These are the corrections needed in the provided files.

@rohitmalhotra1420
Copy link
Collaborator

@abhishek-01k This looks good to me. Please go ahead and make a alpha release for this and update this PR with the alpha release.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 2, 2024

In the package.json file, there are no apparent mistakes or typos. The scripts and dependencies seem to be correctly defined.

In the ChatSection.tsx file, there is a missing closing curly brace for the onChatSelected prop inside the Welcome component.

However, in the yarn.lock file, everything seems to be in order as well. The dependencies and resolutions are correctly specified.

All looks good.

@rohitmalhotra1420 rohitmalhotra1420 merged commit b97cb06 into main Jul 2, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mishramonalisha76 mishramonalisha76 mishramonalisha76 requested changes

@rohitmalhotra1420 rohitmalhotra1420 rohitmalhotra1420 approved these changes

Assignees

@abhishek-01k abhishek-01k

Labels
Quick PR A PR that can be approved before finishing a coffee
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

🐛 [BUG] - [BUMP SDK IN DAPP] - Fix the the modal close behaviour on click outside modal
3 participants
@abhishek-01k @mishramonalisha76 @rohitmalhotra1420