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

fix: share button available in mobile view #1233

Merged
merged 4 commits into from
Oct 17, 2024

Conversation

JeevaRamanathan
Copy link
Contributor

@JeevaRamanathan JeevaRamanathan commented Oct 4, 2024

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    Bug Fix
  • Why was this change needed? (You can also link to an open issue here)
    Address 🐛 Bug Report: No share button on mobile #1227
    image
    Along with the avatar it would be:
    image

Copy link

vercel bot commented Oct 4, 2024

@JeevaRamanathan is attempting to deploy a commit to the Arc53 Team on Vercel.

A member of the Team first needs to authorize it.

@siiddhantt
Copy link
Collaborator

siiddhantt commented Oct 4, 2024

hi! @JeevaRamanathan Thanks for the PR.

this needs to be aligned properly
Screenshot 2024-10-04 at 11 50 06 PM

@JeevaRamanathan
Copy link
Contributor Author

Hi @siiddhantt, based on this #1227 (comment), Profile icon is not on the open source version, if the screenshot that you shared is tested locally by any chance. Could you please confirm?

@siiddhantt
Copy link
Collaborator

oh true it's not on the OSS version, just the positioning then so it's aligned properly. Thanks!

Copy link

vercel bot commented Oct 5, 2024

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

Name Status Preview Comments Updated (UTC)
docs-gpt ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 14, 2024 9:50pm

@JeevaRamanathan
Copy link
Contributor Author

@dartpain, @ManishMadan2882 requesting your review on this

Copy link
Collaborator

@ManishMadan2882 ManishMadan2882 left a comment

Choose a reason for hiding this comment

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

@JeevaRamanathan
In mobile screen the Share button in Navigation is overlapped by another share button in the Conversation component.
Could you please try to have Share button in any one of the components and sync your branch.
Thanks!

@JeevaRamanathan
Copy link
Contributor Author

Sure, let me check

@JeevaRamanathan
Copy link
Contributor Author

Oh yeah, thanks for highlighting @ManishMadan2882, as Share button is used in multiple components I created a separate ShareButton component and used hidden md:block in Conversation component. Is this approach fine?

@JeevaRamanathan
Copy link
Contributor Author

Hi @ManishMadan2882 merge conflict resolved!

@JeevaRamanathan
Copy link
Contributor Author

Hi @ManishMadan2882, would you mind looking into this changes in completing this PR

Copy link
Collaborator

@ManishMadan2882 ManishMadan2882 left a comment

Choose a reason for hiding this comment

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

Just tested on my Playwright emulator for Mobile Safari, looks good !

@dartpain dartpain merged commit 7bf7967 into arc53:main Oct 17, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants