fix: improve layout design for large screens #1231 #1437

RitaDee · 2023-07-26T22:57:47Z

Description

This PR fixes the issue of Odd design for large screens #1231

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

Related Tickets & Documents

Fixes Feature: Odd design for large screens #1231.

Desktop Screenshots:

Before:

After:

Added tests?

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

Added to documentation?

netlify · 2023-07-26T22:57:52Z

✅ Deploy Preview for design-insights ready!

Name	Link
🔨 Latest commit	`2f98364`
🔍 Latest deploy log	https://app.netlify.com/sites/design-insights/deploys/64c7e28690740400088ca5e3
😎 Deploy Preview	https://deploy-preview-1437--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.

netlify · 2023-07-26T22:57:53Z

✅ Deploy Preview for oss-insights ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`2f98364`
🔍 Latest deploy log	https://app.netlify.com/sites/oss-insights/deploys/64c7e285a0f72c0008e063f5
😎 Deploy Preview	https://deploy-preview-1437--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.

github-actions

PR Compliance Checks Passed!

bdougie · 2023-07-26T23:14:48Z

Screen cap before and after please

RitaDee · 2023-07-27T05:35:28Z

@bdougie, done. Thank you but it is yet to pass the checks

OgDev-01 · 2023-07-27T06:18:13Z

Hey @RitaDee, Thanks for taking this on 🍕 .

A few notes after review

The preview shows that the Add to Insight button is now visible on lg screens 👍.

Screen.Recording.2023-07-27.at.07.01.53.mov

in line with setting the width in lg to display the button, we should as well hide the LAST 30 DAYS column of the table to avoid overflow. We should also switch to tailwind styles as that is what is used across the application

components/organisms/Repositories/repositories.tsx

RitaDee · 2023-07-29T21:47:09Z

Closing this as the solution seems a little scope crept to me 😊🫠!

a0m0rajab · 2023-07-30T06:01:28Z

@RitaDee if you want to keep working on this and get some help, I would be more than happy to walk through with you in this PR.

It might be worth taking a look at these codes if you are still interested.

https://github.com/open-sauced/insights/blob/932568ffe78b810b68fa0f053e62c87f7f9ceb9d/components/molecules/RepoRow/repo-row.tsx#L191-L200

https://github.com/open-sauced/insights/blob/932568ffe78b810b68fa0f053e62c87f7f9ceb9d/components/molecules/RepoRow/repo-row.tsx#L263-L268

RitaDee · 2023-07-30T13:08:14Z

Hello @a0m0rajab, I will be glad to receive the walkthrough. Kindly let me know what time works best for you. Thank you

a0m0rajab · 2023-07-31T00:25:06Z

Awesome, to set expectations, this is a bit of a tricky task (you might need to change around 6-7 files till you get it fixed), there is a lot of complexity in it, and that's okay to find it hard, yet it's a bit of a great way to get familiar with the code.

To implement this, you can do the next:

Get Identifier: go to the insight page, use your browser's page inspector, and get the related class/identifier of the item you want to change.
Understand the code: After getting that identifier, you will need to check it from the code and see where it has been used,
Make changes: Since we checked the code, we will make some changes based on our knowledge to see if it works or not,
Review the changes: we need to review the changes and see how the result, and if there is anything else broken,
Repeat the steps: if there is anything broken and not working as expected, we need to redo the whole steps till we get the needed result.

Each iteration will teach you about the code, the logic, and how it works.

As for this task,

Get Identifier

Here is how you can do this:

www_screencapture_com_2023-7-31_03_05.mp4

From this at the end, you can see that I got; flex-1 lg:min-w-[150px] hidden lg:flex

I will go to my code editor right now or GitHub to find these classes.

I am using VsCode so here is how to do it with it:

www_screencapture_com_2023-7-31_03_14.mp4

Here is how to do it with GitHub:

open the repo page
write the search term
check the results

www_screencapture_com_2023-7-31_03_13.mp4

GitHub Result:

https://github.com/search?q=repo%3Aopen-sauced%2Finsights%20flex-1%20lg%3Amin-w-%5B150px%5D%20hidden%20lg%3Aflex&type=code

Understand the code

Since we got the related code right now, we will need to understand it. The first thing to notice is that it has a comment saying it's a column and it's using tailwind:

 {/* Column Last 30 Days */}
        <div className={clsx("flex-1 lg:min-w-[150px] hidden lg:flex justify-center")}>
          {last30days ? <Sparkline data={last30days} width="100%" height={54} /> : "-"}
        </div>

https://github.com/open-sauced/insights/blob/932568ffe78b810b68fa0f053e62c87f7f9ceb9d/components/molecules/ContributorListTableRow/contributor-list-table-row.tsx#L171C1-L175

Tailwind

If you need to become more familiar with it, it's just a short style of writing CSS as a class. It has a special identifier as well. For example:

sm:m-50 lg:w-100

The previous code is converting to: if small screen, then the width should be 50% if large screens, then the width is 100%

https://tailwindcss.com/docs/responsive-design

We can see that the keyword hidden is being used as well. Here we can have three questions:

what would happen if we removed hidden?
When is it hidden, and when it's shown?
How to hide it on the screen that I want

Make changes

Since we get that, we can now change the code and see the results. We can change the lg to xl since we want to hide the things in the large screen and see the result, if it is okay we can move forward.

The result

After doing changes in code, you need to run it on your local machine to test it, get the screen, and see if you need to fix/change anything else.

Repeat

You can repeat the same steps for a 4-5 times till you get the full expected result, again this is a bit tricky task though.

If you need any help or if you feel stuck and like to pair program then I would be happy to do that on discord: contributors channel.

RitaDee · 2023-07-31T02:12:31Z

Thank you, @a0m0rajab I will check it out.

a0m0rajab · 2023-07-31T16:02:54Z

I just had a 30-40 minutes call with @RitaDee to apply the d1d76f1 commit.

Thank you for your time!

As for my opinion, I think that this PR is good right now but, I have two things in consideration:

the feed is a bit out of the scope of the issue
there are some changes that is a bit confusing for review.

pages/feed/index.tsx

components/organisms/Repositories/repositories.tsx

a0m0rajab · 2023-07-31T16:39:40Z

This is how it looks right now:

a0m0rajab

@RitaDee Thank you for your effort and force pushing the PR,

LGTM! 👍

a0m0rajab · 2023-07-31T17:43:49Z

@OgDev-01 the thing you are mentioning is a bit out of scope for this PR, you can check the issue: #1231

The screenshot is a bit misleading

OgDev-01 · 2023-07-31T17:46:01Z

@OgDev-01 the thing you are mentioning is a bit out of scope for this PR, you can check the issue: #1231

The screenshot is a bit misleading

Thanks, I already deleted that... sorry about the misconceptions >:

OgDev-01

LGTM 🍕 🍕 ... Thanks for this fix @RitaDee. Welcome to the community 👏

brandonroberts

🍕

## [1.58.0-beta.3](v1.58.0-beta.2...v1.58.0-beta.3) (2023-07-31) ### 🐛 Bug Fixes * improve layout design for large screens [#1231](#1231) ([#1437](#1437)) ([d8ae808](d8ae808))

github-actions · 2023-07-31T18:30:27Z

🎉 This PR is included in version 1.58.0-beta.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

## [1.58.0](v1.57.0...v1.58.0) (2023-08-02) ### 🧑‍💻 Code Refactoring * merge `authSession` function into `useSession` hook ([#1391](#1391)) ([d6c230d](d6c230d)) ### 🎨 Styles * add width and max width for created at in contributor-profile-tab ([#1429](#1429)) ([5614886](5614886)) * remove irrelevant padding in page divider ([#1427](#1427)) ([5faaa4e](5faaa4e)) ### 🍕 Features * add devcard button to user profile ([#1339](#1339)) ([6a1dbdc](6a1dbdc)) * new contributor highlight card ([#1443](#1443)) ([c88000b](c88000b)) * update redirect to feed page for unauthenticated users ([#1464](#1464)) ([6d8505f](6d8505f)) ### 🐛 Bug Fixes * add checkbox id name if not available based on label ([#1466](#1466)) ([68f66a7](68f66a7)) * add navigation to improve accessibility ([#1436](#1436)) ([d1d85f7](d1d85f7)) * contributor profile tab click state flicker ([#1432](#1432)) ([c9cf8ed](c9cf8ed)) * Deleted page button changed to delete highlight ([#1419](#1419)) ([d502605](d502605)) * fixed bug on chatbot button overlay ([#1420](#1420)) ([2a94583](2a94583)) * hide onboarding button on mobile ([#1460](#1460)) ([f63f240](f63f240)) * Improve Keyboard Accessibility for Notification Icon ([#1435](#1435)) ([05291c0](05291c0)) * improve layout design for large screens [#1231](#1231) ([#1437](#1437)) ([d8ae808](d8ae808)) * misaligned chat button close icon ([#1422](#1422)) ([60f22bd](60f22bd)) * mismatched selection color ([#1430](#1430)) ([8a1d37b](8a1d37b)) * reduce tab font size in contributors profile page ([#1413](#1413)) ([238dc2f](238dc2f)) * show repo filters on initial `feeds` route visit ([#1426](#1426)) ([833ee30](833ee30)) * tab inconsistency in user profile matching the url ([#1403](#1403)) ([f8c6766](f8c6766))

github-actions · 2023-08-02T23:37:55Z

🎉 This PR is included in version 1.58.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

github-actions bot reviewed Jul 26, 2023

View reviewed changes

RitaDee changed the title ~~Fix: Odd design for large screens #1231~~ fix: Odd design for large screens #1231 Jul 27, 2023

OgDev-01 suggested changes Jul 27, 2023

View reviewed changes

components/organisms/Repositories/repositories.tsx Outdated Show resolved Hide resolved

brandonroberts added the requested changes label Jul 27, 2023

a0m0rajab removed the requested changes label Jul 28, 2023

a0m0rajab requested a review from OgDev-01 July 28, 2023 22:36

RitaDee closed this Jul 29, 2023

RitaDee deleted the PR/odd-design branch July 29, 2023 21:47

a0m0rajab mentioned this pull request Jul 31, 2023

Feature: refactor tables #1450

Closed

2 tasks

RitaDee restored the PR/odd-design branch July 31, 2023 13:51

RitaDee reopened this Jul 31, 2023

a0m0rajab reviewed Jul 31, 2023

View reviewed changes

pages/feed/index.tsx Outdated Show resolved Hide resolved

components/organisms/Repositories/repositories.tsx Show resolved Hide resolved

Fix repositories row

2f98364

RitaDee force-pushed the PR/odd-design branch from d1d76f1 to 2f98364 Compare July 31, 2023 16:34

a0m0rajab approved these changes Jul 31, 2023

View reviewed changes

OgDev-01 approved these changes Jul 31, 2023

View reviewed changes

OgDev-01 requested a review from bdougie July 31, 2023 17:48

OgDev-01 added the needs-approval label Jul 31, 2023

brandonroberts approved these changes Jul 31, 2023

View reviewed changes

brandonroberts changed the title ~~fix: Odd design for large screens #1231~~ fix: improve layout design for large screens #1231 Jul 31, 2023

OgDev-01 merged commit d8ae808 into open-sauced:beta Jul 31, 2023
14 checks passed

github-actions bot added the released on @beta label Jul 31, 2023

a0m0rajab mentioned this pull request Jul 31, 2023

Feature: add menu to large screens #1455

Closed

2 tasks

github-actions bot added the released label Aug 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: improve layout design for large screens #1231 #1437

fix: improve layout design for large screens #1231 #1437

RitaDee commented Jul 26, 2023 •

edited

netlify bot commented Jul 26, 2023 •

edited

netlify bot commented Jul 26, 2023 •

edited

github-actions bot left a comment •

edited

bdougie commented Jul 26, 2023

RitaDee commented Jul 27, 2023

OgDev-01 commented Jul 27, 2023

RitaDee commented Jul 29, 2023

a0m0rajab commented Jul 30, 2023

RitaDee commented Jul 30, 2023

a0m0rajab commented Jul 31, 2023

RitaDee commented Jul 31, 2023

a0m0rajab commented Jul 31, 2023

a0m0rajab commented Jul 31, 2023

a0m0rajab left a comment

a0m0rajab commented Jul 31, 2023

OgDev-01 commented Jul 31, 2023

OgDev-01 left a comment

brandonroberts left a comment

github-actions bot commented Jul 31, 2023

github-actions bot commented Aug 2, 2023

fix: improve layout design for large screens #1231 #1437

fix: improve layout design for large screens #1231 #1437

Conversation

RitaDee commented Jul 26, 2023 • edited

Description

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

Related Tickets & Documents

Desktop Screenshots:

Added tests?

Added to documentation?

netlify bot commented Jul 26, 2023 • edited

✅ Deploy Preview for design-insights ready!

netlify bot commented Jul 26, 2023 • edited

✅ Deploy Preview for oss-insights ready!

github-actions bot left a comment • edited

Choose a reason for hiding this comment

bdougie commented Jul 26, 2023

RitaDee commented Jul 27, 2023

OgDev-01 commented Jul 27, 2023

A few notes after review

RitaDee commented Jul 29, 2023

a0m0rajab commented Jul 30, 2023

RitaDee commented Jul 30, 2023

a0m0rajab commented Jul 31, 2023

Get Identifier

Understand the code

Make changes

The result

Repeat

RitaDee commented Jul 31, 2023

a0m0rajab commented Jul 31, 2023

a0m0rajab commented Jul 31, 2023

a0m0rajab left a comment

Choose a reason for hiding this comment

a0m0rajab commented Jul 31, 2023

OgDev-01 commented Jul 31, 2023

OgDev-01 left a comment

Choose a reason for hiding this comment

brandonroberts left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 31, 2023

github-actions bot commented Aug 2, 2023

RitaDee commented Jul 26, 2023 •

edited

netlify bot commented Jul 26, 2023 •

edited

netlify bot commented Jul 26, 2023 •

edited

github-actions bot left a comment •

edited