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

Search improvements and upstream fixes to SPHT #217

Open
agriyakhetarpal opened this issue Nov 8, 2024 · 9 comments
Open

Search improvements and upstream fixes to SPHT #217

agriyakhetarpal opened this issue Nov 8, 2024 · 9 comments

Comments

@agriyakhetarpal
Copy link
Member

Thanks @agriyakhetarpal, this looks really good and in terms of functionality it is working well 🚀

However there can be some UI improvements (I couldn't stop myself comparing it to https://docs.pybamm.org/en/stable/ search bar):

  • We should have some hover over styling property on the search button

  • While search is enabled, I can still scroll the page unlike on the docs

  • Some text to indicate it's keybinding on wider screens

  • It is so fast that I don't feel the necessity of a loading circle but can it take time in some scenario?

  • I'd personally not prefer pitch colors used in the background to keep it familiar with the wesite colors:
    image

  • clear button on searchbar closes it and it should also have some hover effect

  • And this one is also personal - I'd rather keep the icon a little more minimal but it depends upon how you and others also think about it.

Can't wait to see it 😄

EDIT : I see most of it is from the search implemented on scientific-python-hugo-theme so I'd not mind if these points are not addressed too. That being said, I'd still encourage you to see if we can override these properties.

Originally posted by @arjxn-py in #210 (review)

@agriyakhetarpal agriyakhetarpal changed the title Search improvemenrts Search improvements and upstream fixes to SPHT Nov 8, 2024
@Bashamega
Copy link
Contributor

Hello @agriyakhetarpal
I would like to work on this. I think these can simply be overwritten in a CSS file.

@agriyakhetarpal
Copy link
Member Author

Hi @Bashamega, thank you! Please feel free to work on these issues. I'm not sure which of these are still valid and which might have been resolved upstream already. Could you please investigate them? Once done, please send a PR to the Scientific Python Hugo Theme repository with the relevant updates to their styling, as it will benefit everyone who is using the theme, like us. Once those updates are merged and released, we can update our theme submodule with that.

@Bashamega
Copy link
Contributor

Hi @Bashamega, thank you! Please feel free to work on these issues. I'm not sure which of these are still valid and which might have been resolved upstream already. Could you please investigate them? Once done, please send a PR to the Scientific Python Hugo Theme repository with the relevant updates to their styling, as it will benefit everyone who is using the theme, like us. Once those updates are merged and released, we can update our theme submodule with that.

Okay. Thank you

@Bashamega
Copy link
Contributor

I have a question regarding the tasks.
Why is a hover effect needed on the search icon? It isn't an icon

@agriyakhetarpal
Copy link
Member Author

I have a question regarding the tasks. Why is a hover effect needed on the search icon? It isn't an icon

@arjxn-py, it has been a while – is there a chance you remember why you suggested this point?

@Bashamega
Copy link
Contributor

scientific-python/scientific-python-hugo-theme#669
I have opened a PR to fix the clear button hover effect.

@arjxn-py
Copy link
Member

arjxn-py commented Mar 11, 2025

Why is a hover effect needed on the search icon? It isn't an icon

Hey @Bashamega @agriyakhetarpal, yeah its been some time
This is not a hard requirement so feel free to skip it, i remember I just asked this on Agriya's PR since the search icon was looking out of the place as compared to the theme we're using but I'd not be bothered at all to skip it if it's not simple :)
Thanks for picking this up 🚀

@Bashamega
Copy link
Contributor

Why is a hover effect needed on the search icon? It isn't an icon

Hey @Bashamega @agriyakhetarpal, yeah its been some time
This is not a hard requirement so feel free to skip it, i remember I just asked this on Agriya's PR since the search icon was looking out of the place as compared to the theme we're using but I'd not be bothered at all to skip it if it's not simple :)
Thanks for picking this up 🚀

You're welcome 😊
I already created a PR, but there is a problem in it

@arjxn-py
Copy link
Member

arjxn-py commented Mar 11, 2025

Ahh I see, I think to make some work easier - instead of changing the style we can maybe just change Clear to Close

Ideally, it should not close the search bar but if it does let's inform the user :)

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

No branches or pull requests

3 participants