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: wrong background color for search bar in light mode #120

Conversation

nekomamoushi
Copy link
Contributor

PR Checklist

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Display dark background on lightt mode in the search bar

Issue Number: #117

What is the new behavior?

Correct background color for search bar on light mode

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Copy link

netlify bot commented Dec 11, 2023

Deploy Preview for chipper-marshmallow-7b90fb ready!

Name Link
🔨 Latest commit f83c642
🔍 Latest deploy log https://app.netlify.com/sites/chipper-marshmallow-7b90fb/deploys/6579c904441efd000834c11f
😎 Deploy Preview https://deploy-preview-120--chipper-marshmallow-7b90fb.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@geromegrignon
Copy link
Contributor

Can you rebase on latest main branch?

Changes occured meanwhile with the new control flow syntax.

@nekomamoushi
Copy link
Contributor Author

Arf I missed the last update... will rebase asap :)

@nekomamoushi
Copy link
Contributor Author

Hi,

It seems the material component update itself his background in light or dark mode. So there is no need for these classes : bg-white dark:bg-[424242].

Could you confirm it please ?

I still have the background issue if I let these classes. When I remove them, it works fine :)

Copy link
Contributor

@ajitzero ajitzero left a comment

Choose a reason for hiding this comment

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

Hi @nekomamoushi - It seems your local npm modules are outdated, specifically the one for Prettier. Please run npm i and then run npx nx format:write again.

@nekomamoushi
Copy link
Contributor Author

I was wondering what went wrong :)

Thank You

@ajitzero
Copy link
Contributor

No worries! I was on the lookout for issues like this since I just updated Prettier last night (that's what you rebased onto) 😅

@geromegrignon
Copy link
Contributor

Hi,

It seems the material component update itself his background in light or dark mode. So there is no need for these classes : bg-white dark:bg-[424242].

Could you confirm it please ?

I still have the background issue if I let these classes. When I remove them, it works fine :)

The class has been introduced to hide the search results if you scroll.
Otherwise the results are displayed behind the search bar by scrolling. You can try on a mobile screen with 'italy' as a search term.

@nekomamoushi
Copy link
Contributor Author

Hi, thanks for the information. I tried many things to fix this background issue but nothing worked...
Try to target the material class directly, etc... I didn't find a solution.

Do you have any idea that maybe help ? :)

@ajitzero
Copy link
Contributor

ajitzero commented Dec 15, 2023

Try to target the material class directly

First approach: I tried to edit in the browser, and the main changes should be to replace the pt-5 class with top-5 (this causes the overflow) and then set bg-white dark:bg-black (use actual colors, not white/black) in the same tag which already had the class.

But also, (second approach) I think the scroll section shouldn't go to the top; that way, we wouldn't need to set the background in the first place after we replace the absolute positioning of the search text input with a more straightforward flex layout.

@geromegrignon
Copy link
Contributor

closed as removed feature from the new UI.

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.

3 participants