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: Optimize UI for smaller screens (#1220) #1491

Merged

Conversation

Kiryous
Copy link
Contributor

@Kiryous Kiryous commented Jul 30, 2024

Closes #1220

📑 Description

Fix overflow, padding, and other issues that occur on smaller screens (<1280 px):

  • page-container and card-container classes were introduced to unify the paddings
  • min-w-0 added to places where the overflow of flex container happened
  • fallback to one column where appropriate

Known issues:

  • new container classes may not be updated everywhere yet
  • there's room for further changes requiring design decisions; intentionally left them for the follow-up PRs

✅ Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

ℹ Additional Information


Watch Loom with comparison

Copy link

vercel bot commented Jul 30, 2024

@Kiryous is attempting to deploy a commit to the KeepHQ Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

vercel bot commented Aug 11, 2024

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

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
keep ⬜️ Ignored (Inspect) Visit Preview Aug 26, 2024 2:25pm

talboren
talboren previously approved these changes Aug 11, 2024
Copy link
Member

@talboren talboren left a comment

Choose a reason for hiding this comment

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

LGTM!

@talboren
Copy link
Member

@Kiryous it seems like for some reason there is some E2E test failing due to the changes introduced in this branch. Can you please see if you can fix it?

@Kiryous
Copy link
Contributor Author

Kiryous commented Aug 12, 2024

@talboren hey! yeah, I noticed it, but it seems bug in tests rather than bug in code, since I could see the provider's card in headful mode. But maybe I'm wrong. I'm out on vacation till August 23, and could look into it on return.

Matvey-Kuk
Matvey-Kuk previously approved these changes Aug 19, 2024
@Matvey-Kuk Matvey-Kuk enabled auto-merge (squash) August 19, 2024 07:47
@Matvey-Kuk
Copy link
Contributor

@Kiryous we're kinda blocked from merging this while it's breaking tests anyway :(

@talboren
Copy link
Member

@Kiryous @Matvey-Kuk I'm closing this PR for now as it is stale and we can re-open it once @Kiryous is able to fix the small issue here. 🙏🏼

@talboren talboren closed this Aug 25, 2024
auto-merge was automatically disabled August 25, 2024 07:41

Pull request was closed

talboren
talboren previously approved these changes Aug 26, 2024
Copy link
Member

@talboren talboren left a comment

Choose a reason for hiding this comment

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

lgtm

@Kiryous
Copy link
Contributor Author

Kiryous commented Aug 26, 2024

lgtm

@talboren thanks for reviewing and approving. Do not merge yet, please, wanna make a few minor additions

@talboren talboren dismissed their stale review August 26, 2024 07:00

PR author would like to add a few more minor changes

@talboren talboren enabled auto-merge (squash) August 26, 2024 07:01
@talboren talboren self-requested a review August 26, 2024 07:01
Copy link

codecov bot commented Aug 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 30.66%. Comparing base (6d5c759) to head (4b84014).
Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1491      +/-   ##
==========================================
- Coverage   30.68%   30.66%   -0.02%     
==========================================
  Files          54       54              
  Lines        5028     5031       +3     
==========================================
  Hits         1543     1543              
- Misses       3485     3488       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

auto-merge was automatically disabled August 26, 2024 11:48

Head branch was pushed to by a user without write access

@Kiryous
Copy link
Contributor Author

Kiryous commented Aug 26, 2024

@talboren added a fix so tiles of linked providers won't jump on hover. good to go!

@Matvey-Kuk Matvey-Kuk enabled auto-merge (squash) August 26, 2024 12:02
@Kiryous
Copy link
Contributor Author

Kiryous commented Aug 26, 2024

@Matvey-Kuk it seems the merging is blocked because the Vercel deployment is need authorization. Could you please authorize it?

@Matvey-Kuk Matvey-Kuk merged commit 45f7553 into keephq:main Aug 26, 2024
8 checks passed
@Matvey-Kuk
Copy link
Contributor

@Kiryous congrats and thank you for the contribution! <3

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.

Optimize UI for smaller screens
4 participants