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

Added onIconClick to extension search input #9767

Merged
merged 1 commit into from
Nov 1, 2023

Conversation

srietkerk
Copy link
Contributor

When searching for extensions in the Minecraft target, clicking on the magnifying glass icon on the right of the search bar wouldn't do anything. It was unclear to me why it was behaving differently. As far as I could tell, there weren't any flags that would set this in the target itself and when inspecting the html, everything looked the same as it does in arcade, where clicking the icon will kick off a search as expected.

Adding the onIconClick field to the input does change the icon into a button, but I think this is a fine change. I actually think that this modification makes it clearer that the icon can be clicked since it comes with the css that we have defined for common-buttons (pointer and darker background color on hover).

This is a quick video showing how it looks as only an icon and then now as a button:

arcade-search-extensions.mp4

Here's a target in Minecraft: https://minecraft.makecode.com/app/cc86807a84f344ed00a69ca22a3c95f31105b02f-3cdea2d8d6#
This includes some css changes that I made to the button to make it look the same as the other targets. I'm opening another PR in Minecraft to get those changes.

Fixes https://github.com/microsoft/pxt-minecraft/issues/2392

@srietkerk srietkerk requested a review from a team November 1, 2023 18:33
@abchatra
Copy link
Collaborator

abchatra commented Nov 1, 2023

Port to stable.

@srietkerk
Copy link
Contributor Author

Port to stable.

This should just be a cherry-pick from master, right?

@abchatra
Copy link
Collaborator

abchatra commented Nov 1, 2023

Yes

@srietkerk srietkerk merged commit b55b9f4 into master Nov 1, 2023
7 checks passed
@srietkerk srietkerk deleted the srietkerk-extension-search branch November 1, 2023 20:59
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