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

Bug : Fix dark page issue on pro help page #1352

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jagpreetrahi
Copy link

Hello Maintainers,
I have fixed the issues outlined in #1289
Please review the changes and let me know if there are any adjustments required. I would greatly appreciate it if you could merge this pull request after review.
Thank you for your time and effort in maintaining this project!

What kind of change does this PR introduce?

  • Bugfix

Issue Number:
- Closes #1289

Screenshots/videos:

Screenshot (114)

2025-01-21-22-45-16_VcSB1rNR.mp4

If relevant, did you update the documentation?

  • Not Applicable

Summary

  • Updated LinkedIn and GitHub icons to white for better visibility.
  • Changed the TSC label text color to a darker shade for proper contrast.
  • Enhanced the "Reach out" button to improve readability and visibility in dark mode.

Does this PR introduce a breaking change?

No

@jagpreetrahi jagpreetrahi requested a review from a team as a code owner January 21, 2025 17:29
Copy link
Contributor

@Karan-Palan Karan-Palan left a comment

Choose a reason for hiding this comment

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

LGTM, but some checks are failing. Go to files-changed page and check the linting errors. Address these issues and commit the changes.

@Karan-Palan
Copy link
Contributor

@jagpreetrahi try running the tests and build commands before committing your changes, will be helpful for you to find out errors locally

@jagpreetrahi
Copy link
Author

Hii @Karan-Palan , firstly thank you for reviews , I have got an errors where I get , now i have to fix asap, then I 'll quickly commit

@jagpreetrahi
Copy link
Author

Hii @Karan-Palan , Currently I am facing two error related to 'prettier' , the errors looks like
166:24 error Delete ······················· prettier/prettier
188:135 error Delete · prettier/prettier
I am trying to resolve it but everytime i am facing this , can u help me to identifying this error

@Karan-Palan
Copy link
Contributor

@jagpreetrahi in your terminal , type npm run , you will get a bunch of commands

image

Before pushing a commit, try running the tests, building the app and checking if everything is formatted correctly. Since you have a linting error, use npm run lint:fix and push the changes. Hope it helps

@DhairyaMajmudar
Copy link
Member

@jagpreetrahi it seems your format checks are failing pls. fix them by running the command yarn format:fix in your console.

Copy link

github-actions bot commented Jan 22, 2025

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
website ✅ Ready (View Log) Visit Preview 9b1cadf

Copy link

codecov bot commented Jan 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (ee9538b) to head (9b1cadf).

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #1352   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines          373       373           
  Branches        94        94           
=========================================
  Hits           373       373           

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

@jagpreetrahi
Copy link
Author

Hii @Karan-Palan @DhairyaMajmudar , thanks for helping me to resolve this error, now it is fix and all test cases are successfully passed.

@jagpreetrahi
Copy link
Author

Hey @DhairyaMajmudar , can you review my pull request again , whenever you have get time.

@jagpreetrahi
Copy link
Author

Hi @Karan-Palan, I would appreciate it if you could review my pull request again. If it produces valuable output, I would be grateful if you could merge it. Thank you!

@Karan-Palan
Copy link
Contributor

@jagpreetrahi I'm just a contributor here. Please don't tag maintainers until it's at least a week of inactivity. I understand as contributors we're eager to contribute but it might irritate the maintainers. This is a mistake I did as well, hence informing for your further contributions

@jagpreetrahi
Copy link
Author

Ok @Karan-Palan , I got it

Copy link
Member

@DhairyaMajmudar DhairyaMajmudar left a comment

Choose a reason for hiding this comment

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

Thanks for the fix @jagpreetrahi left some comments then we're good to go.

Copy link
Member

Choose a reason for hiding this comment

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

Changes in this file are irrelevant pls. remove them

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.

🐛 Bug: More dark mode problems - pro-help
3 participants