Fixed UI issues for Sidebar #634

abirc8010 · 2024-10-01T20:55:08Z

Brief Title

This PR fixes UI issues for Sidebar making it responsive

Acceptance Criteria fulfillment

Introduced a variable isSmallScreen to detect if the width of screen is <= 780px and adjusts the style of the component accordingly
For small screens , styled the Sidebar with position: absolute and covering entire width by setting width to 100%

Fixes # (issue)
#629
#685

Video/Screenshots

Screencast.from.2024-10-02.02-07-54.webm

Popup Mode:

popup.webm

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

abirc8010 · 2024-10-01T21:00:40Z

@Spiral-Memory do I need to test it on some more cases ?

Spiral-Memory · 2024-10-01T21:01:13Z

Really loved the thorough testing done by you in the attached video, I'll review the PR and test myself once and let you know.

Thank you so much for your contribution ❤️

Spiral-Memory · 2024-10-01T21:03:55Z

Once go to design variants, modern design and check if popup mode is working fine after this change and attach a video

abirc8010 · 2024-10-01T21:15:38Z

I've attached the popup mode video ,
I haven't made any changes for the popup mode for now though , I introduced a conditional statement so that the styles are applied for the Sidebar component only

abirc8010 · 2024-10-04T14:56:18Z

Hey @Spiral-Memory , are any more tests required ?

Spiral-Memory · 2024-10-04T15:02:24Z

I haven't tested it yet, let the pr preview auto deployment feature get implemented, afterwards, I'll test and let you know if any changes are required

Spiral-Memory · 2024-10-06T07:57:14Z

Hey @abirc8010,

I noticed two issues. Please fix them:

In normal mode, the sidebar title is properly visible, but when switching to a smaller screen (with less height), the title is not visible. Ensure that in both modes, the title aligns at the same height.

While in dev mode, the switch does not happen automatically when changing screen sizes. I have to manually open and close the sidebar to reflect the changes, which is not the intended behavior.

abirc8010 · 2024-10-06T08:01:50Z

While in dev mode, the switch does not happen automatically when changing screen sizes. I have to manually open and close the sidebar to reflect the changes, which is not the intended behavior.

To fix this issue, I have to add resize event listener , will that be ok ?

Spiral-Memory · 2024-10-06T08:04:42Z

While in dev mode, the switch does not happen automatically when changing screen sizes. I have to manually open and close the sidebar to reflect the changes, which is not the intended behavior.

To fix this issue, I have to add resize event listener , will that be ok ?

But why do we need listeners ? Why can't we use media queries to handle it properly ?

abirc8010 · 2024-10-06T08:33:59Z

But why do we need listeners ? Why can't we use media queries to handle it properly ?

The Sidebar component accepts a style prop in the form of a css object, which makes it difficult to implement media queries effectively. That's why I am considering using a resize event listener instead .
Do you have any suggestions?

Spiral-Memory · 2024-10-06T14:37:35Z

But why do we need listeners ? Why can't we use media queries to handle it properly ?

The Sidebar component accepts a style prop in the form of a css object, which makes it difficult to implement media queries effectively. That's why I am considering using a resize event listener instead . Do you have any suggestions?

It is not recommended to use event listeners in this use case. Try using only media queries. You can still write the CSS in the style.js file and pass it. Try to figure out a solution using media queries.

abirc8010 · 2024-10-07T20:05:49Z

@Spiral-Memory , I have fixed the issues

Spiral-Memory

The approach seems fine; you are trying to add it to the global styles. However, why can't we add similar media queries and pass them directly to the sidebars, without overriding class styles globally?

abirc8010 · 2024-10-11T16:19:41Z

I noticed that the Sidebar component from the embeddedChat ui-element only accepts style prop as JavaScript objects, which doesn't allow for media queries. As a result, I had to apply the styles using the classname in global styles .

Spiral-Memory · 2024-10-11T16:25:16Z

As far as I'm aware, you can set media queries in a style object as well, or try using the css attribute with Emotion/React. I'll have to look into it to confirm.

Spiral-Memory

Approving to test !

Spiral-Memory · 2024-10-11T16:29:03Z

@devanshkansagra

Could you please check, if we can use media queries in style object ?.

Spiral-Memory · 2024-10-11T16:39:33Z

Unnecessary spacing !

abirc8010 · 2024-10-11T17:35:18Z

Hey @Spiral-Memory , I fixed the issue

Spiral-Memory

Approving to test !

abirc8010 · 2024-10-22T04:16:03Z

Approving to test !

@Spiral-Memory do I need to make any changes?

Spiral-Memory · 2024-11-09T05:48:07Z

Ok @devanshkansagra
No worries.. I'll publish an unofficial package and test

Spiral-Memory · 2024-11-10T10:54:06Z

packages/react/src/views/Message/Message.js

@@ -1,6 +1,6 @@
 import React, { memo, useContext } from 'react';
 import PropTypes from 'prop-types';
-import { format } from 'date-fns';
+import { format, set } from 'date-fns';


importing set ?

I think it was automatically imported by vs code . It doesn't have a use case in this context, I'll remove it.

remove this too

Spiral-Memory · 2024-11-10T10:54:49Z

packages/react/src/views/Message/Message.styles.js

@@ -75,7 +75,7 @@ export const getMessageDividerStyles = (theme) => {
      line-height: 1rem;
      position: relative;
      display: flex;
-      z-index: 1000;
+      z-index: 1;


I'm a little afraid about z-index changes. It might disturb the existing flow in some situation. Is this required?

@Spiral-Memory , actually due to a high value of z-index, the message divider was appearing on top of the sidebar like the image shown , so I had to decrease the value.

Spiral-Memory · 2024-12-15T10:14:24Z

Hi @abirc8010

I really want to merge this PR. Could you please address the concerns I raised regarding the z-index and the unused set?

I understand that the z-index is necessary, but could you modify it so that the new z-index is applied only at a specific breakpoint? When I initially set it up, I followed a specific set of z-index values to maintain consistency. That's why I can't merge it with a z-index of 1 across all screens.

abirc8010 · 2024-12-15T11:35:19Z

Hey @Spiral-Memory I reverted the zIndex for all except for the message divider , I used media query to reduce the zIndex for lower screen size to prevent it from appearing over the sidebar

Spiral-Memory · 2024-12-15T11:38:16Z

packages/react/src/views/MessageAggregators/common/MessageAggregator.js

@@ -70,7 +70,7 @@ export const MessageAggregator = ({
      iconName={iconName}
      searchProps={searchProps}
      onClose={() => setExclusiveState(null)}
-      style={{ padding: 0 }}
+      style={{ zIndex: 1 }}


These are still present

abirc8010 · 2024-12-15T11:53:42Z

@Spiral-Memory I have reverted the zIndexes which were already present , can I add zIndex for the ViewComponent for smaller screens ?
Some of the areas are getting disturbed if I don't adjust the zIndex

Spiral-Memory · 2024-12-15T11:55:26Z

For smaller screen, you can specifically add zIndex according to the need.. i just don't want to let zindex changes to have affect in normal view

abirc8010 · 2024-12-15T12:25:56Z

Hey @Spiral-Memory I have made the required changes

packages/react/src/views/GlobalStyles.js

Spiral-Memory · 2024-12-22T08:26:25Z

Looks good to me. Thanks a lot, @abirc8010

Please add some good padding to the sidebars as well, notice that it is very less between the avatar and the side corner but you can include that in another PR. Since this has been pending for a long time, I am merging it.

Fixed UI issues for Sidebar

4bd0e53

Spiral-Memory force-pushed the make-sidebar-responsive branch from 775f8fd to 4bd0e53 Compare October 6, 2024 07:05

Spiral-Memory approved these changes Oct 6, 2024

View reviewed changes

Merge branch 'develop' into make-sidebar-responsive

6d34acc

devanshkansagra approved these changes Oct 6, 2024

View reviewed changes

Fixed sidebar height issue

67ac114

abirc8010 requested a review from Spiral-Memory October 10, 2024 08:35

Spiral-Memory reviewed Oct 11, 2024

View reviewed changes

Spiral-Memory approved these changes Oct 11, 2024

View reviewed changes

Removed unnecessary spacing

4f779b9

Spiral-Memory approved these changes Oct 20, 2024

View reviewed changes

Merge branch 'develop' into make-sidebar-responsive

7355815

Merge branch 'develop' into make-sidebar-responsive

eda168b

Spiral-Memory reviewed Nov 10, 2024

View reviewed changes

abirc8010 added 3 commits November 11, 2024 11:01

Merge branch 'develop' into make-sidebar-responsive

2956300

Merge branch 'develop' into make-sidebar-responsive

b0d26cc

Merge branch 'develop' into make-sidebar-responsive

8779fea

abirc8010 and others added 4 commits December 15, 2024 16:21

Merge branch 'develop' into make-sidebar-responsive

8f4990d

Merge branch 'develop' into make-sidebar-responsive

f5967d9

Revert zIndex changes

1d9aea9

Linting fix

4c16eed

Run prettier

76a5e71

Spiral-Memory reviewed Dec 15, 2024

View reviewed changes

Remove zIndex

4438840

Adjusted zIndex for ViewComponent

c1fc25d

Fix sidebar width

8a5590d

abirc8010 mentioned this pull request Dec 15, 2024

BUG: Sidepanel Width Expands Unexpectedly with Content #685

Closed

Spiral-Memory approved these changes Dec 16, 2024

View reviewed changes

Spiral-Memory reviewed Dec 16, 2024

View reviewed changes

packages/react/src/views/GlobalStyles.js Outdated Show resolved Hide resolved

abirc8010 and others added 2 commits December 17, 2024 01:01

Remove global css for sidebar

3f7d144

Merge branch 'develop' into make-sidebar-responsive

60ef15d

Spiral-Memory approved these changes Dec 17, 2024

View reviewed changes

Spiral-Memory merged commit 9a3b7ac into RocketChat:develop Dec 22, 2024
4 checks passed

github-actions bot added a commit that referenced this pull request Dec 22, 2024

Remove deployment for PR #634

0cd732b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed UI issues for Sidebar #634

Fixed UI issues for Sidebar #634

abirc8010 commented Oct 1, 2024 •

edited

Loading

abirc8010 commented Oct 1, 2024

Spiral-Memory commented Oct 1, 2024

Spiral-Memory commented Oct 1, 2024 •

edited

Loading

abirc8010 commented Oct 1, 2024

abirc8010 commented Oct 4, 2024

Spiral-Memory commented Oct 4, 2024

Spiral-Memory commented Oct 6, 2024

abirc8010 commented Oct 6, 2024

Spiral-Memory commented Oct 6, 2024 •

edited

Loading

abirc8010 commented Oct 6, 2024

Spiral-Memory commented Oct 6, 2024

abirc8010 commented Oct 7, 2024

Spiral-Memory left a comment

abirc8010 commented Oct 11, 2024

Spiral-Memory commented Oct 11, 2024

Spiral-Memory left a comment

Spiral-Memory commented Oct 11, 2024

Spiral-Memory commented Oct 11, 2024 •

edited

Loading

abirc8010 commented Oct 11, 2024 •

edited

Loading

Spiral-Memory left a comment

abirc8010 commented Oct 22, 2024

Spiral-Memory commented Nov 9, 2024

Spiral-Memory Nov 10, 2024

abirc8010 Nov 10, 2024

Spiral-Memory Dec 15, 2024

Spiral-Memory Nov 10, 2024

abirc8010 Nov 10, 2024

Spiral-Memory commented Dec 15, 2024

abirc8010 commented Dec 15, 2024

Spiral-Memory Dec 15, 2024

abirc8010 commented Dec 15, 2024

Spiral-Memory commented Dec 15, 2024

abirc8010 commented Dec 15, 2024

Spiral-Memory commented Dec 22, 2024

Fixed UI issues for Sidebar #634

Fixed UI issues for Sidebar #634

Conversation

abirc8010 commented Oct 1, 2024 • edited Loading

Brief Title

Acceptance Criteria fulfillment

Video/Screenshots

abirc8010 commented Oct 1, 2024

Spiral-Memory commented Oct 1, 2024

Spiral-Memory commented Oct 1, 2024 • edited Loading

abirc8010 commented Oct 1, 2024

abirc8010 commented Oct 4, 2024

Spiral-Memory commented Oct 4, 2024

Spiral-Memory commented Oct 6, 2024

abirc8010 commented Oct 6, 2024

Spiral-Memory commented Oct 6, 2024 • edited Loading

abirc8010 commented Oct 6, 2024

Spiral-Memory commented Oct 6, 2024

abirc8010 commented Oct 7, 2024

Spiral-Memory left a comment

Choose a reason for hiding this comment

abirc8010 commented Oct 11, 2024

Spiral-Memory commented Oct 11, 2024

Spiral-Memory left a comment

Choose a reason for hiding this comment

Spiral-Memory commented Oct 11, 2024

Spiral-Memory commented Oct 11, 2024 • edited Loading

abirc8010 commented Oct 11, 2024 • edited Loading

Spiral-Memory left a comment

Choose a reason for hiding this comment

abirc8010 commented Oct 22, 2024

Spiral-Memory commented Nov 9, 2024

Spiral-Memory Nov 10, 2024

Choose a reason for hiding this comment

abirc8010 Nov 10, 2024

Choose a reason for hiding this comment

Spiral-Memory Dec 15, 2024

Choose a reason for hiding this comment

Spiral-Memory Nov 10, 2024

Choose a reason for hiding this comment

abirc8010 Nov 10, 2024

Choose a reason for hiding this comment

Spiral-Memory commented Dec 15, 2024

abirc8010 commented Dec 15, 2024

Spiral-Memory Dec 15, 2024

Choose a reason for hiding this comment

abirc8010 commented Dec 15, 2024

Spiral-Memory commented Dec 15, 2024

abirc8010 commented Dec 15, 2024

Spiral-Memory commented Dec 22, 2024

abirc8010 commented Oct 1, 2024 •

edited

Loading

Spiral-Memory commented Oct 1, 2024 •

edited

Loading

Spiral-Memory commented Oct 6, 2024 •

edited

Loading

Spiral-Memory commented Oct 11, 2024 •

edited

Loading

abirc8010 commented Oct 11, 2024 •

edited

Loading