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

Feat: Adding mobile resposiveness #943

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

Stpriyasharma
Copy link
Contributor

@Stpriyasharma Stpriyasharma commented Feb 1, 2025

Brief Title Adding responsiveness to mobile devices for modals

Fixes #942

Video/Screenshots

Recording.2025-02-01.120329.mp4

PR Test Details

Note: The PR will be ready for live testing at https://rocketchat.github.io/EmbeddedChat/pulls/pr-943 after approval.

@Spiral-Memory
Copy link
Collaborator

Can we please have smaller Emoji box with no search in mobile

@Spiral-Memory
Copy link
Collaborator

Spiral-Memory commented Feb 2, 2025

Also, update your branch with latest changes in develop branch :

It has nothing to do with playwright, why playwright changes are being reflected here

@@ -14,6 +14,25 @@ const getAttachmentPreviewStyles = () => {
width: 100%;
`,

modal: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the same code is being repeated so many times, can we just make modal itself mobile responsive from ui-elements

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah i have tried but changes are not reflecting from there

@dhairyashiil
Copy link
Contributor

Can we please have smaller Emoji box with no search in mobile

Hello @Spiral-Memory , actually, on mobile, we should have a search feature, right? Even more so than on larger screens, because mobile screens are smaller, and it's harder to find emojis there (based on my personal experience). I think we should keep the search feature on mobile as well.

@Stpriyasharma
Copy link
Contributor Author

I have made the emoji picker small in my recent commit

@Spiral-Memory
Copy link
Collaborator

Can we please have smaller Emoji box with no search in mobile

Hello @Spiral-Memory , actually, on mobile, we should have a search feature, right? Even more so than on larger screens, because mobile screens are smaller, and it's harder to find emojis there (based on my personal experience). I think we should keep the search feature on mobile as well.

You are correct @dhairyashiil
But, actually in mobile version, the emoji should take the place of keyboard, with search feature

Not with the popups 😥

@Stpriyasharma
Copy link
Contributor Author

Can we please have smaller Emoji box with no search in mobile

Hello @Spiral-Memory , actually, on mobile, we should have a search feature, right? Even more so than on larger screens, because mobile screens are smaller, and it's harder to find emojis there (based on my personal experience). I think we should keep the search feature on mobile as well.

You are correct @dhairyashiil But, actually in mobile version, the emoji should take the place of keyboard, with search feature

Not with the popups 😥

should i remove search bar?

@Spiral-Memory
Copy link
Collaborator

Spiral-Memory commented Feb 2, 2025

@Stpriyasharma

We've to look for a emoji alternative, for now, keep it smaller with search

@Spiral-Memory
Copy link
Collaborator

See how it is implemented in rocket.chat

@Stpriyasharma @dhairyashiil

Screenshot_2025-02-02-16-00-50-80_e4424258c8b8649f6e67d283a50a2cbc.jpg

@Spiral-Memory
Copy link
Collaborator

Anyone can work on that, as a separate PR

In this PR, keep search but make the emoji popup smaller for all screens whichever looks good

@dhairyashiil
Copy link
Contributor

Anyone can work on that, as a separate PR

In this PR, keep search but make the emoji popup smaller for all screens whichever looks good

Okay, Got it. Thanks! I will look into it then

@Stpriyasharma
Copy link
Contributor Author

Recording.2025-02-02.160003.mp4

i have made changes in recent commit

@Spiral-Memory
Copy link
Collaborator

Great @Stpriyasharma

Can you please make the requested changes too ?

There is a lot of code repetition.. there is a package named ui-elements.. that have the modal component I think, you can directly make it mobile responsive

Instead of repeating code everywhere modal is used

@Stpriyasharma
Copy link
Contributor Author

ok @Spiral-Memory . i will make the changes

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.

feat: mobile responsiveness for modals
3 participants