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.

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

Responsive Chat + Reactions #1531

Closed
wants to merge 14 commits into from
Closed

Responsive Chat + Reactions #1531

wants to merge 14 commits into from

Conversation

HarshRajat
Copy link
Contributor

@HarshRajat HarshRajat commented May 15, 2024

Fixes:

  • Chat responsiveness SDK component container div
  • Enables chat users to go back to Chats tab once they accept a chat request
  • Adds a TODO comment on possible culprits for timeout errors happening on localhost
  • Bumps the package to alpha uiweb for supporting reactions

Copy link

vercel bot commented May 15, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
push-dapp ❌ Failed (Inspect) May 24, 2024 9:00am

Copy link

In the provided files:

  1. In the index.html file:

    • The comments inside <!-- ... --> tags are not properly formatted. They should be enclosed within <p> or other HTML tags to appear correctly in the browser.
    • There is a missing closing <html> tag at the end of the file.
  2. In the package.json file:

    • The dependencies section seems to include ellipses (...) which should be replaced with actual dependencies for the sake of completeness.
    • There is a missing closing curly brace } at the end of the dependencies section before the "devDependencies" section.
    • The "devDependencies" section is not shown completely, hence it's unclear if there are any issues or not.
    • The indentation in the script commands can be made consistent for better readability.
  3. In the public/robots.txt file:

    • The robots.txt file appears to be correctly formatted with a user-agent and sitemap directives.
  4. In the public/sitemap.txt file:

    • The sitemap.txt file looks well-structured with a list of URLs for different sections of the website.
  5. The src/sections/chat/ChatSection.tsx, yarn.lock files were not provided, so they couldn't be reviewed.

Overall, the HTML and configuration files have some minor issues that need attention, but there are no critical errors. If the missing files are provided for review, a more thorough evaluation can be done.

Copy link

It seems you provided multiple files for the code review but only the content of index.html file is visible. Could you please also provide the content of other files mentioned (package.json, public/robots.txt, public/sitemap.txt, src/sections/chat/ChatSection.tsx, yarn.lock) so that a thorough review can be conducted? Thank you.

Copy link
Collaborator

@rohitmalhotra1420 rohitmalhotra1420 left a comment

Choose a reason for hiding this comment

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

@HarshRajat

  • Could you let me know what does this pr exactly fix.
  • Fix the alpha deployment file changes.
  • Resolve the merge conflicts on this one

@HarshRajat HarshRajat changed the title responsive-fix-wrt-sdk Responsive Chat + Reactions May 21, 2024
Copy link

In index.html:

  1. Missing closing angle bracket for the meta tag: <meta name="theme-color" content="#ffffff">. It should be <meta name="theme-color" content="#ffffff">.
  2. Missing opening tag for <link> with rel="manifest" and href="/site.webmanifest".

In package.json:

  1. The "dependencies" object is missing a closing curly brace at the end.

In public/robots.txt:
All looks good.

In public/sitemap.txt:
All looks good.

In src/components/InitState.tsx:
All looks good.

In src/components/ViewDelegateeItem.jsx:
All looks good.

In src/hooks/useResolveWeb3Name.ts:
All looks good.

In src/modules/chat/ChatModule.tsx:

  1. The formatting of the component function seems to be incorrect. Some lines are not wrapped properly.

In src/modules/inbox/InboxModule.tsx:
All looks good.

In src/sections/chat/ChatSection.tsx:
All looks good.

In src/singletons/ChannelsDataStore.js:
All looks good.

In yarn.lock:
All looks good.

Copy link

github-actions bot commented May 21, 2024

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-05-27 16:18 UTC

package.json Outdated
@@ -34,9 +34,9 @@
"@metamask/eth-sig-util": "^4.0.0",
"@mui/icons-material": "^5.8.4",
"@mui/material": "^5.5.0",
"@pushprotocol/restapi": "1.7.18",
"@pushprotocol/restapi": "1.7.19",
Copy link
Collaborator

Choose a reason for hiding this comment

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

https://push-protocol.github.io/push-dapp/pr-preview/pr-1531/channels

The Developer section and the channels page keeps on loading on the preview URL. It is working on other PR previews.

I see a localhost error in the console. My guess is this this is related to the restapi package. Right?

Screenshot 2024-05-22 at 4 01 23 PM

Copy link
Contributor Author

@HarshRajat HarshRajat May 22, 2024

Choose a reason for hiding this comment

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

Weird, I don't think restapi should change the behavior, reverting robots, sitemap and index to prod config to see if that somehow fixes this.

Copy link

All looks good.

Copy link

All looks good.

Copy link

There are a few issues in the provided code snippets:

  1. In the .github/workflows/preview.yml file:
  • The yarn install and yarn build:pr:preview commands in the Install and Build job are not properly formatted. They should be placed in separate run steps.
  1. In the .github/workflows/preview.yml file:
  • In the Deploy preview job, there is a redundant step that uses rossjrw/[email protected]. It seems to be a duplicate step and should be removed.
  1. In the .github/workflows/preview.yml file:
  • There is a missing - name: Delete Preview before the if condition in the job step. It looks like there was an attempt to add a new step but it is incomplete and the existing step goes directly to the conditions.
  1. In the package.json file:
  • There is an extra , at the end of the dependencies list before the ellipsis which is not needed.
  1. In the src/components/InitState.tsx file:
  • There is a typo in the function call pushUser.channel.info() should be checked for any possible errors or misspellings.
  1. In the src/components/ViewDelegateeItem.jsx file:
  • The import statement for LoaderSpinner imports it from a wrong path. It should be corrected to the correct path where LoaderSpinner is located.
  1. In the src/components/ViewDelegateeItem.jsx file:
  • The import statement for Blockies is misspelled. It should be corrected to import Blockies from 'components/reusables/BlockiesIdenticon';.

After addressing these issues, the code should be more correct. If you need further assistance or clarification, feel free to ask.

Copy link

All looks good.

Copy link

All looks good.

@rohitmalhotra1420
Copy link
Collaborator

rohitmalhotra1420 commented May 27, 2024

Closing this PR as we have a created a fresh PR by checking out to a new branch chat-reactions from this branch: #1581

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants