Skip to content

Conversation

@Darshilp326
Copy link
Contributor

Proposed changes (including videos or screenshots)

All selected hide system messages are now in vertical Bar.

system-message-issue.mp4

Issue(s)

Closes #20357

Steps to test or reproduce

Further comments

Copy link
Member

@MartinSchoeler MartinSchoeler left a comment

Choose a reason for hiding this comment

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

Requested some changes.

Tip: try out a bit with the maxWidth property

Thanks!

@MartinSchoeler MartinSchoeler self-assigned this Jan 25, 2021
@Darshilp326
Copy link
Contributor Author

@MartinSchoeler Working!

@Darshilp326
Copy link
Contributor Author

System-messages-changes.mp4

@MartinSchoeler Please review:)

Copy link
Member

@MartinSchoeler MartinSchoeler left a comment

Choose a reason for hiding this comment

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

Looking way better! Let me give you another tip for making this PR perfect:

Almost all of our components can receive css properties directly as props, you only need to camel case the css property name, e.g. <Element maxWidth='100%'></Element> with this Rocket.Chat is creating CSS rules at runtime and squeezing a little bit more performance out of its components, also this is the pattern that we are following with our React Components.

If you want to know more, you can check our Components documentation (we call it 'Fuselage').

@Darshilp326
Copy link
Contributor Author

@MartinSchoeler Done!

@Darshilp326
Copy link
Contributor Author

@MartinSchoeler Please review:)

Copy link
Member

@MartinSchoeler MartinSchoeler left a comment

Choose a reason for hiding this comment

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

Looks good to me now!

@ggazzo ggazzo merged commit 73db41b into RocketChat:develop Feb 2, 2021
@sampaiodiego sampaiodiego mentioned this pull request Feb 28, 2021
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.

[BUG]Selecting more than 3 hide system messages, content goes out of the screen.

3 participants