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.

Sign up for GitHub

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

Jump to bottom

style: tightened up trending content for smaller screens #690

Merged
merged 1 commit into from
Jan 29, 2025
Merged

style: tightened up trending content for smaller screens #690

merged 1 commit into from
Jan 29, 2025
+164 −138

Conversation

jtucholski
Copy link
Contributor

Description of the change

This PR tries to tighten up some of the cards used on the Trending Content screen when viewed on regular laptop screens. The existing spacing was left untouched for xl screens.

Screenshots (laptop screen)

Before

127 0 0 1_trending-content-laptop-before

After

127 0 0 1_trending-content-laptop-after

Screenshots (large monitor)

Before

127 0 0 1_trending-content-monitor-before

After

127 0 0 1_trending-content-monitor-after

@jtucholski jtucholski requested a review from calisio January 24, 2025 20:59
@jtucholski
Copy link
Contributor Author

@calisio Here are a few changes to see if it can help improve the layout of Trending Content on smaller displays. I'd appreciate any feedback you have or tweaks that need to be made.

carddev81
carddev81 approved these changes Jan 27, 2025
View reviewed changes
Copy link
Contributor

@carddev81 carddev81 left a comment

Choose a reason for hiding this comment

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

Added a note to the PR, nothing changed to cause this, I just noticed this while testing.

frontend/src/Pages/StudentLayer1.tsx Show resolved Hide resolved
calisio
calisio approved these changes Jan 28, 2025
View reviewed changes
Copy link
Contributor

@calisio calisio left a comment

Choose a reason for hiding this comment

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

I think it looks good. Interestingly, my computer is an xl layout, so I only see this when I make my screen smaller. Once Rich's comment is fixed it's good to merge!

@jtucholski
Copy link
Contributor Author

@calisio @carddev81 I've added an ExpandableCardGrid component and applied it to the Helpful Links section seen in Trending Content. The same component was applied wherever we had Featured Content or Favorite Libraries visible.

@calisio
Copy link
Contributor

calisio commented Jan 28, 2025

Tried it out and it looks good- seems like there are a few conflicts but once those are fixed its good to merge

carddev81
carddev81 approved these changes Jan 28, 2025
View reviewed changes
Copy link
Contributor

@carddev81 carddev81 left a comment

Choose a reason for hiding this comment

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

Looks Great!

@jtucholski jtucholski force-pushed the trending-content-layout branch from db072d3 to 3e56efe Compare January 29, 2025 11:43
@jtucholski
Copy link
Contributor Author

@carddev81 Something weird is going on with this conflict. It says the issue is in ExpandableCardGrid but that's because the changes took place to FeaturedContent which I removed and refactored into an ExpandableCardGrid component.

It looks like FeaturedContent was updated to include the Search modals. Is that something that needs to exist in the FeaturedContent component?

@jtucholski jtucholski force-pushed the trending-content-layout branch from 3e56efe to 89258a7 Compare January 29, 2025 18:02
@jtucholski jtucholski merged commit d320fca into UnlockedLabs:main Jan 29, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carddev81 carddev81 carddev81 approved these changes

@calisio calisio calisio approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

UI: Fix Layout and Display Issues on Trending Content Page (Increase Responsiveness)
3 participants
@jtucholski @calisio @carddev81