fix search icon contrast issues

[yiningwang11]

fix #660
I modified the theme colour of the search icon, now it looks like this:

	light mode	dark mode
unhovered
hovered

[filiptronicek]

Nice one-liner! I hope this won't have unintended consequences 😄

Is there a way to maybe make a new Theme just for that one search component? I think if we changed it here we'd have to test the whole Web UI for color changes and side-effects.

[yiningwang11]

Thanks for the comment!
Let me see if there's a way to separate the theme for this component. I'm thinking maybe I should create a new theme and apply it to the search icon button individually?

[filiptronicek]

I think that would be the better option here indeed. Looping in @amvanbaren, maybe you have some ideas :)

[amvanbaren]

I see the main issue has already been fixed.
There are 2 more things I noticed:

• You don't have to pass themeType through props. You can use const { pageSettings } = useContext(MainContext); instead. For example:

  openvsx/webui/src/pages/admin-dashboard/namespace-admin.tsx

  Line 21 in 5df9b74

  const { pageSettings, service, user, handleError } = useContext(MainContext);

• You can define style classes instead of using inline styles, e.g. iconButtonLight and iconButtonDark.
  

  openvsx/webui/src/pages/extension-list/extension-list-searchfield.tsx

  Lines 29 to 37 in 5df9b74

  	iconButton: {
  	backgroundColor: theme.palette.secondary.main,
  	borderRadius: '0 4px 4px 0',
  	padding: theme.spacing(1),
  	transition: 'all 0s',
  	'&:hover': {
  	filter: 'invert(100%)',
  	}
  	},

  
  And then you can use it like this:
  

  openvsx/webui/src/pages/extension-detail/extension-detail.tsx

  Lines 348 to 352 in 5df9b74

  	const classes = this.props.classes;
  	const warningClass = themeType === 'dark' ? classes.warningDark : classes.warningLight;
  	const themeClass = themeType === 'dark' ? classes.darkTheme : classes.lightTheme;
  	if (!extension.verified) {
  	return <Paper className={`${classes.banner} ${warningClass} ${themeClass}`}>

[amvanbaren]

Looks way better! One nitpick: you can replace

const prefersDarkMode = pageSettings?.themeType === 'dark' ? true : false;

with

const searchIconClass = pageSettings?.themeType === 'dark' ? classes.searchIconDark : classes.searchIconLight;

and then use that to style the SearchIcon, so that you only have one ternary (condition ? x : y) statement.

@filiptronicek Does it fix the contrast issues?

[filiptronicek]

The contrast issues are solved.

[amvanbaren]

Ok, LGTM. @yiningwang11 can you squash your commits, so that this PR gets merged into master as 1 commit?

[yiningwang11]

Hello @amvanbaren ,
I am following this guide here to do the squash.
I did git stash and then git rebase -i HEAD~6. Then I changed the pick to squash as suggested. After modifying the commit message I see a message on the terminal as:

[detached HEAD 3caf568] Ready to deploy
 Author: amvanbaren <[email protected]>
 Date: Tue Apr 18 13:42:54 2023 +0300
 5 files changed, 123 insertions(+), 77 deletions(-)
 create mode 100644 server/src/main/java/org/eclipse/openvsx/migration/GenerateKeyPairJobService.java
Successfully rebased and updated refs/heads/contrastIcon.

I noticed that there must be something wrong with my process because the commits on this branch are not being squashed and the message I got above is clearly not what I expected.
I suspect this is because one of my commits e6a1de6 is still going through the checks.
I'm kindly asking if there is a way to fix or undo what I have done. Apologies for the mess that I brought :(

[amvanbaren]

@yiningwang11 Luckily you haven't pushed contrastIcon yet. You can reset your local branch to reflect the remote one: https://www.freecodecamp.org/news/git-reset-origin-how-to-reset-a-local-branch-to-remote-tracking-branch/#reset.

Then you should rebase the last 5 commits (git rebase -i HEAD~5):

• pick cd6bb87
• squash 8c19e96, 0696508, 4f0eeaf and e6a1de6.

You can see your list of commits here: https://github.com/eclipse/openvsx/pull/730/commits.
The changes of the resulting (squashed) commit should be the same as https://github.com/eclipse/openvsx/pull/730/files.

[yiningwang11]

Hello @amvanbaren ,
Thank you for your help!
Following the guide, I have reset my local branch contrastIcon by doing git reset --hard origin/contrastIcon to reflect the remote one and it now looks good. I am now able to proceed to do the rebase for squash.

[filiptronicek]

Awesome stuff!

I think the next nice step is to maybe add a transition to the button, but maybe that's for another PR :)