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(fuselage): Responsive CardControls #1443

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

Barrylimarti
Copy link

Proposed changes (including videos or screenshots)

Added flex-wrap in the CSS of Cards for CardControl component. Things that have been kept in mind are:-

  1. Wrap is not essential for horizontal cards in desktop view.
  2. Wrap is enabled for horizontal cards in mobile view.
  3. In every other case, flex-wrap is essential for responsive behaviour.
  4. Made Carcontrol into a box component for better integration.
responsive.mov

Issue(s)

Closes #1440 #31614

Further comments

There are other ways to go around it but this felt appropriate and fixes the issue.

Copy link

changeset-bot bot commented Sep 4, 2024

🦋 Changeset detected

Latest commit: e2a79ef

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@rocket.chat/fuselage Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@CLAassistant
Copy link

CLAassistant commented Sep 4, 2024

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@casalsgh casalsgh requested a review from a team September 4, 2024 15:59
Barrylimarti and others added 2 commits September 19, 2024 02:49
Box is not needed. A better way would be to use the mixin for conditional css.

Co-authored-by: Henrique Guimarães Ribeiro <[email protected]>
Co-authored-by: Henrique Guimarães Ribeiro <[email protected]>
@Barrylimarti
Copy link
Author

Hey Henrique! I just saw a repetition of code caused by previous commit. I fixed that in the latest commit. Now everything looks fine to me. Can you please check?

@Barrylimarti
Copy link
Author

Hey @rique223! All the changes requested by you are done from my side. Let me know if you are looking for something else.

rique223
rique223 previously approved these changes Oct 20, 2024
Copy link
Contributor

@rique223 rique223 left a comment

Choose a reason for hiding this comment

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

Lgtm

@juliajforesti juliajforesti changed the title fix(fuselage): Responsive CardControls fix(fuselage): Responsive CardControls Oct 22, 2024
.changeset/purple-shirts-boil.md Outdated Show resolved Hide resolved
packages/fuselage/src/components/Card/CardControls.tsx Outdated Show resolved Hide resolved
Co-authored-by: Júlia Jaeger Foresti <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Card: CardControls is not completely responsive
4 participants