fix: Feed page responsiveness #1490

RitaDee · 2023-08-05T04:03:03Z

Description

Fixes #1453

Fixes #1336

Duplicate to Issue #1336

What type of PR is this? (check all applicable)

Related Tickets & Documents

#1336

Mobile & Desktop Screenshots/Recordings

Before:

Screen.Recording.2023-07-31.at.19.11.01.mov

After:

Screen.Recording.2023-08-05.at.04.59.09.mov

Added tests?

👍 yes
🙅 no, because they aren't needed
🙋 no, because I need help

Added to documentation?

[optional] Are there any post-deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

netlify · 2023-08-05T04:03:09Z

✅ Deploy Preview for oss-insights ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`c376ebf`
🔍 Latest deploy log	https://app.netlify.com/sites/oss-insights/deploys/64d014648db0b800084932a4
😎 Deploy Preview	https://deploy-preview-1490--oss-insights.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify · 2023-08-05T04:03:17Z

✅ Deploy Preview for design-insights ready!

Name	Link
🔨 Latest commit	`c376ebf`
🔍 Latest deploy log	https://app.netlify.com/sites/design-insights/deploys/64d01464c682b10008f01b67
😎 Deploy Preview	https://deploy-preview-1490--design-insights.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

babblebey · 2023-08-05T09:35:46Z

Reviewing this @RitaDee... in the meantime do append the Fixes term to the duplicate issue too so It can allow it automatically close on merge this PR.

This way you should have...

Fixes #1453

Fixes #1336

RitaDee · 2023-08-05T09:41:15Z

@babblebey Done.

babblebey

Great stuff @RitaDee, but your implementation isn't quite there yet... I have made the diagram below to guide you, it suggests some classes you should remove and ones to add...

Tasks

Append the container and px-2 md:px-16 classes to the main element i.e. the first purple area in the diagram
Append/update the stated classes and remove the striked ones from the next nested div.
Do the same for the other divs.
Remove all fixed width from the Left (userCard) div and Right (filter) div children divs replacing them with w-full to allow them inherit width from their respective parent divs.

Expected Behaviour

The Central (feedInput form and feeds) div should have a flexible width, taking all available spaces in the center
The Left (userCard) div should take 24% width of the total viewport initially, but when screen size scales down below 1280px i.e. tailwind xl, its display should be none.
The Right (filter) div should also take 24% width of the total viewport initially, but when screen size scales down below 1280px i.e. tailwind xl, its width should increase to 30%, at thesame point when the Left (userCard) div is no longer in display.
When screen size scales down below 1024px i.e. tailwind lg the Right (filter) div display at this point should also be none leaving only the Central (feedInput form and feeds) div on screen.

Potential Bottlenecks

You might experience weird behaviour in the width of the Left (userCard) div and Right (filter) div children divs, as seen below..

This is the intention of the Task No. 4, (setting w-full for child components) but you may not be able to fix some of this at the /feed page level, requiring you to go into the source components instead. You can just skip going into the source components for now, do the things you can within the /feed page for now...

Let's see the result of your Implementation first, then we can go fixing for the source component if needed.

We anticipate your screencast/demo after this!

bdougie

I noticed some of the things @babblebey mentioned. Marking requested changes for other reviewers.

RitaDee · 2023-08-05T18:45:31Z

Thank you, @babblebey for providing the diagram and the specific tasks to make the /feed page responsive. I have carefully reviewed the requirements, and I'll make the necessary changes as suggested.

RitaDee · 2023-08-06T22:01:05Z

I acknowledge the suggestions provided by @babblebey. Thank you

Update:

I have improved the layout and styles of the /feed page to achieve a responsive design as outlined in the tasks provided. The changes focus on the main, left, and right sections, ensuring they adapt gracefully to different screen sizes.

I applied the container and px-2 md:px-16 classes to the main element, allowing proper padding and responsiveness. Also, used the appropriate tailwind class to achieve this.

Current behaviour:

Screen.Recording.2023-08-06.at.22.50.22.mov

bdougie

Thanks

## [1.59.0-beta.5](v1.59.0-beta.4...v1.59.0-beta.5) (2023-08-07) ### 🐛 Bug Fixes * Feed page responsiveness ([#1490](#1490)) ([67662b2](67662b2)) ### 🍕 Features * onboarding auto fetch timezone ([#1488](#1488)) ([ae5cdd7](ae5cdd7))

This reverts commit 67662b2.

babblebey

Great work hey @RitaDee,

We have some few things to address.

babblebey · 2023-08-07T13:43:24Z

layouts/profile.tsx

@@ -8,7 +8,9 @@ const ProfileLayout = ({ children }: { children: React.ReactNode }) => {
    <div className="flex flex-col min-h-screen">
      <TopNav />
      <div className="page-container flex grow flex-col items-center">
-        <main className="flex pb-16 w-full flex-1 flex-col items-center bg-light-slate-2">{children}</main>
+        <main className="container px-2 flex md:px-16 pb-16 w-full flex-1 flex-col items-center bg-light-slate-2">


REGRESSION!!!

I see that the changes to the main occured in the ProfileLayout component, this will lead to Regression in other components where the ProfileLayout is integrated. See pages/component where regression occurs below with screenshots

The /user/[username] page integrates the ProfileLayout and it as regressed: https://deploy-preview-1490--oss-insights.netlify.app/user/babblebey/highlights

The /user/setting page is the second component that integrates the ProfileLayout and it as also regressed: https://deploy-preview-1490--oss-insights.netlify.app/user/settings

This is due to the classes (container, px-2 and md:px-16) that we moved up into the component.

FIX!!

Suggested change

<main className="container px-2 flex md:px-16 pb-16 w-full flex-1 flex-col items-center bg-light-slate-2">

<main className="flex pb-16 w-full flex-1 flex-col items-center bg-light-slate-2">

We will have to remove the classes (container, px-2 and md:px-16), from there moving it back to its initial place in the child div.

babblebey · 2023-08-07T13:44:04Z

pages/feed/index.tsx

-      >
-        <div className="flex-col flex-1 hidden gap-6 mt-12 md:flex">
+      <div className="w-full gap-[2rem] justify-center flex flex-col md:gap-6 xl:gap-16 pt-12 md:flex-row" ref={topRef}>
+        <div className="flex-col flex-1 xl:flex hidden gap-6 mt-12">


Suggested change

<div className="flex-col flex-1 xl:flex hidden gap-6 mt-12">

<div className="w-[24%] flex-col flex-none xl:flex hidden gap-6 mt-12">

You are missing a width and flex-none from the Left (userCard) div.

With flex-none, the goal is to size the Left (userCard) div according to the width (that you will have to set to 24%), rendering it fully inflexible: so it neither shrinks nor grows in relation to its parent flex container.

In the absence of these we are have non equal width on the Right (Filter) Div and Left (userCard) div

babblebey · 2023-08-07T13:44:24Z

pages/feed/index.tsx

@@ -284,7 +281,7 @@ const Feeds: WithPageLayout<HighlightSSRProps> = (props: HighlightSSRProps) => {
            <FollowingHighlightWrapper selectedFilter={selectedRepo} emojis={emojis} />
          </TabsContent>
        </Tabs>
-        <div className="hidden gap-6 mt-10 md:flex-1 md:flex md:flex-col">
+        <div className="md:hidden gap-6 mt-10 md:flex-1 lg:flex md:flex-col">


Suggested change

<div className="md:hidden gap-6 mt-10 md:flex-1 lg:flex md:flex-col">

<div className="md:hidden gap-6 mt-10 w-[30%] lg:w-[24%] flex-none lg:flex md:flex-col">

You are also missing a responsive width and flex-none from the Right (Filter) Div. for the same reason as Left (userCard) div.

...and

Suggested change

<div className="md:hidden gap-6 mt-10 md:flex-1 lg:flex md:flex-col">

<div className="hidden gap-6 mt-10 md:flex-1 lg:flex flex-col">

Tailwind uses a mobile-first breakpoint system.

This means that unprefixed classes (like hidden) take effect on all screen sizes, while prefixed one (like lg:flex) only take effect at the specified breakpoint (lg in this case) and above.

So Right Filter Div will be hidden normally on all screen sizes and it will only be in display again at lg (desktop) and above i.e. lg:flex.

So you needn't specify a responsive class of md:hidden since its gonna be hidden normally till lg.

babblebey · 2023-08-07T13:45:28Z

pages/feed/index.tsx

-        ref={topRef}
-      >
-        <div className="flex-col flex-1 hidden gap-6 mt-12 md:flex">
+      <div className="w-full gap-[2rem] justify-center flex flex-col md:gap-6 xl:gap-16 pt-12 md:flex-row" ref={topRef}>


Suggested change

<div className="w-full gap-[2rem] justify-center flex flex-col md:gap-6 xl:gap-16 pt-12 md:flex-row" ref={topRef}>

<div className="w-full gap-[2rem] justify-center flex flex-col pt-12 md:flex-row" ref={topRef}>

The responsive/prefixed gap classes are not affecting anything at those breakpoints (i.e. xl and md), because the flex items the points are already reduced leaving the gap with nothing to space out.

So we can remove them.

babblebey · 2023-08-07T13:45:49Z

pages/feed/index.tsx

          {user && (
-            <div>
+            <div className="md:w-1/2 lg:w-1/3">


Suggested change

<div className="md:w-1/2 lg:w-1/3">

<div className="w-full">

This should only take full width of the parent div, not some fraction of it at breakpoints.

babblebey · 2023-08-07T13:46:43Z

components/atoms/UserCard/user-card.tsx

@@ -18,13 +18,13 @@ const UserCard = ({ username, name, meta, loading }: UserCardProps) => {
  const avatarUrl = getAvatarByUsername(username);

  return (
-    <div className="w-full pb-6 border rounded-lg bg-light-slate-1 border-zinc-200">
+    <div className="pb-6 border bg-light-slate-1 w-max rounded-xl border-zinc-200">


Suggested change

<div className="pb-6 border bg-light-slate-1 w-max rounded-xl border-zinc-200">

<div className="pb-6 border bg-light-slate-1 w-full rounded-lg border-zinc-200">

Return the full width for the user-card and the border-radius component.

Why though!?

The width w-full for size inheritance reason

The Border radius rounded-lg for uniformity reason, looking at all other sibling components/divs on the Right (Filter) Div and Left (userCard) div.

babblebey · 2023-08-07T13:46:53Z

components/atoms/UserCard/user-card.tsx

      {loading ? (
        <div className="flex items-center justify-center h-32 w-72">
          <Spinner className="mt-6 " />
        </div>
      ) : (
-        <div className="flex flex-col items-center gap-6 px-6 ">
+        <div className="flex flex-col items-center gap-6 px-9">


I'm very curious to know why you thought to increase this padding!?

## [1.59.0](v1.58.0...v1.59.0) (2023-08-08) ### 🧑‍💻 Code Refactoring * replace supabase/ui in design system typography components ([#1438](#1438)) ([38cfb30](38cfb30)) ### 🍕 Features * add a user notifications page ([#1478](#1478)) ([022dc69](022dc69)) * add conditional routing to single highlight dialog close action ([#1473](#1473)) ([1341cba](1341cba)) * add github link to profile ([#1459](#1459)) ([d42bc6d](d42bc6d)) * add support for highlight.new ([#1487](#1487)) ([3daa5c0](3daa5c0)) * improved the UX on the feeds page and scroll behaviour ([#1506](#1506)) ([31c1593](31c1593)) * onboarding auto fetch timezone ([#1488](#1488)) ([ae5cdd7](ae5cdd7)) ### 🐛 Bug Fixes * Feed page responsiveness ([#1490](#1490)) ([67662b2](67662b2)) * feeds page typography and styles improvements ([#1467](#1467)) ([a3b289e](a3b289e)) * on page reload Insights page redirecting to Dashboard ([#1517](#1517)) ([397c36e](397c36e)) * update environment variable for Sentry ([#1521](#1521)) ([56ac14b](56ac14b)) * update release workflow to use GitHub app for semantic versioning ([#1484](#1484)) ([3f1ce84](3f1ce84)) * update user interest logo error for machine learning ([#1474](#1474)) ([a286eda](a286eda)) * uses session to update user info for notifications check ([#1486](#1486)) ([60d787e](60d787e))

RitaDee added 2 commits August 5, 2023 04:34

Fix width to accommodate all screen size

d363f5f

Make UserCard Responsive in Flexbox Layout

c07d2c1

Merge branch 'beta' into feed-page

61907c3

babblebey reviewed Aug 5, 2023

View reviewed changes

bdougie requested changes Aug 5, 2023

View reviewed changes

bdougie added the requested changes label Aug 5, 2023

RitaDee added 3 commits August 6, 2023 21:34

Add container and px-2 md:px-16 classes to main element

84654b7

Enhance the behaviour of the Tabs component by adding the 'grow' class

20644ee

Fix feed page responsiveness

c376ebf

bdougie approved these changes Aug 7, 2023

View reviewed changes

bdougie added needs review PRs that need review from core engineering team and removed requested changes labels Aug 7, 2023

bdougie mentioned this pull request Aug 7, 2023

feat: improved the UX on the feeds page and scroll behaviour #1506

Merged

19 tasks

bdougie merged commit 67662b2 into open-sauced:beta Aug 7, 2023

babblebey added a commit to babblebey/insights that referenced this pull request Aug 7, 2023

Revert "fix: Feed page responsiveness (open-sauced#1490)"

c64ca7b

This reverts commit 67662b2.

babblebey mentioned this pull request Aug 7, 2023

revert: "fix: Feed page responsiveness" URGENT!!! #1513

Closed

babblebey reviewed Aug 7, 2023

View reviewed changes

This was referenced Aug 7, 2023

fix: Profile layout regression #1514

Closed

fix: enhance feed layout responsiveness #1516

Closed

babblebey mentioned this pull request Aug 8, 2023

Bug: we cant see the sidebars on small screens #1510

Closed

2 tasks

RitaDee mentioned this pull request Aug 10, 2023

fix: enhance feed layout responsiveness #1538

Closed

19 tasks

takanome-dev mentioned this pull request Aug 11, 2023

Bug: Profile header is squashed due to new margins #1543

Closed

2 tasks

RitaDee mentioned this pull request Aug 14, 2023

fix: enhance feed layout responsiveness #1553

Merged

19 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Feed page responsiveness #1490

fix: Feed page responsiveness #1490

RitaDee commented Aug 5, 2023 •

edited

Loading

netlify bot commented Aug 5, 2023 •

edited

Loading

netlify bot commented Aug 5, 2023 •

edited

Loading

babblebey commented Aug 5, 2023 •

edited

Loading

RitaDee commented Aug 5, 2023

babblebey left a comment •

edited

Loading

bdougie left a comment •

edited

Loading

RitaDee commented Aug 5, 2023

RitaDee commented Aug 6, 2023

bdougie left a comment

babblebey left a comment

babblebey Aug 7, 2023

babblebey Aug 7, 2023 •

edited

Loading

babblebey Aug 7, 2023

babblebey Aug 7, 2023

babblebey Aug 7, 2023

babblebey Aug 7, 2023

babblebey Aug 7, 2023

	<main className="container px-2 flex md:px-16 pb-16 w-full flex-1 flex-col items-center bg-light-slate-2">
	<main className="flex pb-16 w-full flex-1 flex-col items-center bg-light-slate-2">

	<div className="flex-col flex-1 xl:flex hidden gap-6 mt-12">
	<div className="w-[24%] flex-col flex-none xl:flex hidden gap-6 mt-12">

	<div className="md:hidden gap-6 mt-10 md:flex-1 lg:flex md:flex-col">
	<div className="md:hidden gap-6 mt-10 w-[30%] lg:w-[24%] flex-none lg:flex md:flex-col">

	<div className="md:hidden gap-6 mt-10 md:flex-1 lg:flex md:flex-col">
	<div className="hidden gap-6 mt-10 md:flex-1 lg:flex flex-col">

	<div className="w-full gap-[2rem] justify-center flex flex-col md:gap-6 xl:gap-16 pt-12 md:flex-row" ref={topRef}>
	<div className="w-full gap-[2rem] justify-center flex flex-col pt-12 md:flex-row" ref={topRef}>

	<div className="pb-6 border bg-light-slate-1 w-max rounded-xl border-zinc-200">
	<div className="pb-6 border bg-light-slate-1 w-full rounded-lg border-zinc-200">

fix: Feed page responsiveness #1490

fix: Feed page responsiveness #1490

Conversation

RitaDee commented Aug 5, 2023 • edited Loading

Description

What type of PR is this? (check all applicable)

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings

Added tests?

Added to documentation?

[optional] Are there any post-deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

netlify bot commented Aug 5, 2023 • edited Loading

✅ Deploy Preview for oss-insights ready!

netlify bot commented Aug 5, 2023 • edited Loading

✅ Deploy Preview for design-insights ready!

babblebey commented Aug 5, 2023 • edited Loading

RitaDee commented Aug 5, 2023

babblebey left a comment • edited Loading

Choose a reason for hiding this comment

Tasks

Expected Behaviour

Potential Bottlenecks

bdougie left a comment • edited Loading

Choose a reason for hiding this comment

RitaDee commented Aug 5, 2023

RitaDee commented Aug 6, 2023

Update:

Current behaviour:

bdougie left a comment

Choose a reason for hiding this comment

babblebey left a comment

Choose a reason for hiding this comment

babblebey Aug 7, 2023

Choose a reason for hiding this comment

REGRESSION!!!

FIX!!

babblebey Aug 7, 2023 • edited Loading

Choose a reason for hiding this comment

babblebey Aug 7, 2023

Choose a reason for hiding this comment

babblebey Aug 7, 2023

Choose a reason for hiding this comment

babblebey Aug 7, 2023

Choose a reason for hiding this comment

babblebey Aug 7, 2023

Choose a reason for hiding this comment

Why though!?

babblebey Aug 7, 2023

Choose a reason for hiding this comment

RitaDee commented Aug 5, 2023 •

edited

Loading

netlify bot commented Aug 5, 2023 •

edited

Loading

netlify bot commented Aug 5, 2023 •

edited

Loading

babblebey commented Aug 5, 2023 •

edited

Loading

babblebey left a comment •

edited

Loading

bdougie left a comment •

edited

Loading

babblebey Aug 7, 2023 •

edited

Loading