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: improve the light-theme #2256

Closed
wants to merge 14 commits into from
Closed

Conversation

CBID2
Copy link
Collaborator

@CBID2 CBID2 commented Feb 4, 2024

Fixes Issue

Closes #2247

Changes proposed

This PR changes the website's light theme, making it easier for people to navigate the site. Here's what's been done so far:

  • Made the border of the searchbar's input darker and thicker (see the new version of the first screenshot for reference.)
  • Made the border of the breadcrumbs darker and thicker (see the second screenshot for reference.)
  • Add shadowbox and border to card-list and footer(see third screenshot for reference)

Screenshots

  1. Search input having a dark input
    screenshot of search input with a darker border
    Update here's a new version:
    new version of light theme

  2. Breadcrumbs having a darker and thicker border
    made the breadcrumb's light theme darker and thicker
    Update here's an new version of the breadcrumb
    new version of breadcrumbs

  3. Footer and card list
    footer and card

Note to reviewers

This is a work-in-progress

Copy link

vercel bot commented Feb 4, 2024

Someone is attempting to deploy a commit to a Personal Account owned by @rupali-codes on Vercel.

@rupali-codes first needs to authorize it.

@github-actions github-actions bot added bug Something isn't working priority: medium Modifying an existing feature labels Feb 4, 2024
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you, CBID2, for creating this pull request and contributing to LinksHub! 💗

The maintainers will review this Pull Request and provide feedback as soon as possible! 😇
We appreciate your patience and contribution, Keep up the great work! 😀

Copy link

vercel bot commented Feb 4, 2024

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

Name Status Preview Comments Updated (UTC)
linkshub ❌ Failed (Inspect) Mar 27, 2024 3:30pm

@Anmol-Baranwal
Copy link
Collaborator

@CBID2

I know the draft PR isn't ready, but I would suggest these changes.

  1. Even if you want a border in dark theme, use the light border that is used in the cards as shown.

image

  1. and the same for light background. Use a little light border if possible (use opacity).
    It shouldn't be the reason of distraction.

image

@CBID2
Copy link
Collaborator Author

CBID2 commented Feb 4, 2024

@CBID2

I know the draft PR isn't ready, but I would suggest these changes.

  1. Even if you want a border in dark theme, use the light border that is used in the cards as shown.

image

  1. and the same for light background. Use a little light border if possible (use opacity).

It shouldn't be the reason of distraction.

image

@Anmol-Baranwal, so...something like this: https://github.com/rupali-codes/LinksHub/blob/main/components/Cards/Card.tsx
border-light-white

@Anmol-Baranwal
Copy link
Collaborator

@Anmol-Baranwal, so...something like this: https://github.com/rupali-codes/LinksHub/blob/main/components/Cards/Card.tsx border-light-white

For light, I'm not sure if there is any. But for dark, using the one on the cards will avoid mismatch of theme.
You can check locally.

@sarava338
Copy link
Contributor

In Dark theme, we have very dark color in background. But in Light theme, We did not. So I hope, the very light color in background and swap the with card's background color will do the trick.

Dark Theme
image

Light Theme
image

@CBID2
Copy link
Collaborator Author

CBID2 commented Feb 4, 2024

In Dark theme, we have very dark color in background. But in Light theme, We did not. So I hope, the very light color in background and swap the with card's background color will do the trick.

Dark Theme

image

Light Theme

image

So…you suggest that I use the dark theme’s background as a border for the light one @sarava338?

@Anmol-Baranwal
Copy link
Collaborator

So…you suggest that I use the dark theme’s background as a border for the light one @sarava338?

He is saying that in dark theme -> The cards are light color compared to the background color.

But it's the opposite in the light theme, so if we make background complete white and cards as the background color (which is lighter one). Then this problem would be fixed.

@rupali-codes
Don't do this without consulting rupali.

@sarava338
Copy link
Contributor

So…you suggest that I use the dark theme’s background as a border for the light one @sarava338?

He is saying that in dark theme -> The cards are light color compared to the background color.

But it's the opposite in the light theme, so if we make background complete white and cards as the background color (which is lighter one). Then this problem would be fixed.

@rupali-codes
Don't do this without consulting rupali.

Yes. And in the Light theme, the little dark color has to be a little darker.

@CBID2
Copy link
Collaborator Author

CBID2 commented Feb 4, 2024

So…you suggest that I use the dark theme’s background as a border for the light one @sarava338?

He is saying that in dark theme -> The cards are light color compared to the background color.

But it's the opposite in the light theme, so if we make background complete white and cards as the background color (which is lighter one). Then this problem would be fixed.

@rupali-codes Don't do this without consulting rupali.

Ooh just saw this @Anmol-Baranwal after adding the new changes (see the updated photos in the screenshot section of my PR's form). Hopefully, @rupali-codes won't mind the changes.

Copy link
Owner

@rupali-codes rupali-codes left a comment

Choose a reason for hiding this comment

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

I don't see any changes except the border on searchbar

This PR
image

Main site
image

@CBID2
Copy link
Collaborator Author

CBID2 commented Feb 5, 2024

I don't see any changes except the border on searchbar

This PR

image

Main site

image

It's a work-in-progress @rupali-codes. Still trying to figure out how to do the other parts.

@rupali-codes
Copy link
Owner

@CBID2 alright, let us know if you need any help

@CBID2
Copy link
Collaborator Author

CBID2 commented Feb 9, 2024

@CBID2 alright, let us know if you need any help

Hey @rupali-codes and @Anmol-Baranwal, I’m struggling with adding the border to the card components. Can you help me please?

@rupali-codes
Copy link
Owner

@CBID2 alright, let us know if you need any help

Hey @rupali-codes and @Anmol-Baranwal, I’m struggling with adding the border to the card components. Can you help me please?

where you wanna add border?

@CBID2
Copy link
Collaborator Author

CBID2 commented Feb 10, 2024

@CBID2 alright, let us know if you need any help

Hey @rupali-codes and @Anmol-Baranwal, I’m struggling with adding the border to the card components. Can you help me please?

where you wanna add border?

The card components @rupali-codes

@rupali-codes
Copy link
Owner

@CBID2 alright, let us know if you need any help

Hey @rupali-codes and @Anmol-Baranwal, I’m struggling with adding the border to the card components. Can you help me please?

where you wanna add border?

The card components @rupali-codes

alright, it should be in components/cards/card.tsx file, then in the main div add border classes.

What else are you struggling with?

@CBID2
Copy link
Collaborator Author

CBID2 commented Feb 11, 2024

@CBID2 alright, let us know if you need any help

Hey @rupali-codes and @Anmol-Baranwal, I’m struggling with adding the border to the card components. Can you help me please?

where you wanna add border?

The card components @rupali-codes

alright, it should be in components/cards/card.tsx file, then in the main div add border classes.

What else are you struggling with?

Hi @rupali-codes. I added the border to the main div like you suggested and nothing shows
Did I do something wrong?

@rupali-codes
Copy link
Owner

@CBID2 alright, let us know if you need any help

Hey @rupali-codes and @Anmol-Baranwal, I’m struggling with adding the border to the card components. Can you help me please?

where you wanna add border?

The card components @rupali-codes

alright, it should be in components/cards/card.tsx file, then in the main div add border classes.
What else are you struggling with?

Hi @rupali-codes. I added the border to the main div like you suggested and nothing shows Did I do something wrong?

You've misplaced the double quotes

image

It should be <div className="card-body border-light-silver border-2 border-opacity-75">

@Anmol-Baranwal
Copy link
Collaborator

@CBID2
This is still in draft. Are the changes done.

@CBID2
Copy link
Collaborator Author

CBID2 commented Feb 15, 2024

@CBID2

This is still in draft. Are the changes done.

No I have to do the footer @Anmol-Baranwal

@rupali-codes
Copy link
Owner

@CBID2
This is still in draft. Are the changes done.

No I have to do the footer @Anmol-Baranwal

you need any help Chrissy?

@CBID2
Copy link
Collaborator Author

CBID2 commented Feb 18, 2024

@CBID2

This is still in draft. Are the changes done.

No I have to do the footer @Anmol-Baranwal

you need any help Chrissy?

Not yet @rupali-codes

@CBID2 CBID2 marked this pull request as ready for review February 21, 2024 03:13
@CBID2
Copy link
Collaborator Author

CBID2 commented Mar 3, 2024

Made the changes @aftabrehan

aftabrehan
aftabrehan previously approved these changes Mar 3, 2024
Copy link
Owner

@rupali-codes rupali-codes left a comment

Choose a reason for hiding this comment

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

This issue isn't getting fixed, been long ha,
when you try to fix it for light theme it gets weird for dark theme

image

image

@CBID2 please let me know if you need any help, wanna close this ASAP

@CBID2
Copy link
Collaborator Author

CBID2 commented Mar 3, 2024

This issue isn't getting fixed, been long ha,

when you try to fix it for light theme it gets weird for dark theme

image

image

@CBID2 please let me know if you need any help, wanna close this ASAP

Ok can you help @rupali-codes

@rupali-codes
Copy link
Owner

@CBID2 yes of course, where are you getting troubles?

@CBID2
Copy link
Collaborator Author

CBID2 commented Mar 3, 2024

I'm not sure how to make it less weird looking in light mode @rupali-codes. From the photos you posted, it looks fine.

@CBID2
Copy link
Collaborator Author

CBID2 commented Mar 3, 2024

What do you think @Anmol-Baranwal?

@Anmol-Baranwal
Copy link
Collaborator

What do you think @Anmol-Baranwal?

I've no problem if others are okay with it.

@CBID2
Copy link
Collaborator Author

CBID2 commented Mar 4, 2024

@rupali-codes, I really think the theme looks fine.

@CBID2
Copy link
Collaborator Author

CBID2 commented Mar 7, 2024

@rupali-codes, I really think we should merge this.

@rupali-codes
Copy link
Owner

@rupali-codes, I really think we should merge this.

Can you remove the border for dark theme and keep it only for light theme?

@CBID2
Copy link
Collaborator Author

CBID2 commented Mar 7, 2024

@rupali-codes, I really think we should merge this.

Can you remove the border for dark theme and keep it only for light theme?

Ok

@CBID2
Copy link
Collaborator Author

CBID2 commented Mar 15, 2024

Hey @rupali-codes. Can you do another deployment? I made the changes to light theme but I can't see them locally.

This reverts commit 21c3ee3.
@CBID2
Copy link
Collaborator Author

CBID2 commented Mar 24, 2024

@rupali-codes, can you fix the deploy please? It failed for some odd reason.

Update
I found the error ↓
terminal-error
But I'm not quite sure how to solve it. Can you help?

@Anmol-Baranwal
Copy link
Collaborator

@CBID2
Should I create a new PR for this? It's urgent because the current live light theme (live) is far from good.

@CBID2
Copy link
Collaborator Author

CBID2 commented Apr 12, 2024

@CBID2

Should I create a new PR for this? It's urgent because the current live light theme (live) is far from good.

Ok @Anmol-Baranwal

@CBID2 CBID2 closed this Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority: medium Modifying an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] improvements in light and dark theme
5 participants