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

Fixed Arrow Key Selection of Search Suggestion #202

Merged
merged 12 commits into from
Nov 22, 2024
Merged

Conversation

itz-rj-here
Copy link
Collaborator

Closes #183
Bug fix requested by @ceskyDJ

There is still a issue. I can't able to understand how can I use the same bg color of pointer hover color and the arrow key selection color.
Here is the screenshot of the issue:-
image

Need some help on fixing this issue.

@itz-rj-here itz-rj-here marked this pull request as draft November 20, 2024 16:33
@itz-rj-here
Copy link
Collaborator Author

@XengShi Can you check if this works on other browsers until I try to fix the bg color please :o

@itz-rj-here
Copy link
Collaborator Author

Bruh chatgpt also giving wrong suggestion.
He gave me this -_-

// Apply styles for active and hovered items
const style = document.createElement('style');
style.innerHTML = `
    .resultItem.active {
        background-color: #f0f0f0;  /* Change to the desired active background color */
        color: #0000ff;  /* Change to the desired active text color */
        cursor: pointer;  /* Change the cursor to pointer */
    }
    .resultItem:hover {
        background-color: #f0f0f0;  /* Change to the desired hover background color */
        color: #0000ff;  /* Change to the desired hover text color */
        cursor: pointer;  /* Change the cursor to pointer */
    }
`;
document.head.appendChild(style);

@itz-rj-here
Copy link
Collaborator Author

Any ideas guys?

@itz-rj-here
Copy link
Collaborator Author

@prem-k-r can you atleast help me on this selection color issue please 🥺

@prem-k-r
Copy link
Collaborator

prem-k-r commented Nov 20, 2024

wait, i am checking the code in my pc

@prem-k-r
Copy link
Collaborator

ok done, i'll raise pr for it

meanwhile, there is an issue, after visible result( first 5) it is not shifting/scrolling.. it is moving up/down but not visible

@itz-rj-here
Copy link
Collaborator Author

Ok let me look into it too.

@prem-k-r
Copy link
Collaborator

prem-k-r commented Nov 20, 2024

Also we have to see if mouse cursor is within resultBox, then keyboard selection must be unhighlighted

@XengShi
Copy link
Owner

XengShi commented Nov 20, 2024

ok done, i'll raise pr for it

meanwhile, there is an issue, after visible result( first 5) it is not shifting/scrolling.. it is moving up/down but not visible

I noticed one more bug.
When 'Ai tools' is enabled and opened/expended. If you disable the Ai tools toggle shortcuts just disappear.

Hint :
When disabling Ai tools toggle it should first close or collapse [AI tools] button

IMG_20241120_234851.png

@XengShi XengShi marked this pull request as ready for review November 20, 2024 18:21
@itz-rj-here
Copy link
Collaborator Author

Bruh It's not ready for review.

@itz-rj-here itz-rj-here marked this pull request as draft November 20, 2024 18:23
@itz-rj-here
Copy link
Collaborator Author

Someone of you need to fix the highlight thingy when we will let this pr to be merged.

@XengShi
Copy link
Owner

XengShi commented Nov 20, 2024

@XengShi Can you check if this works on other browsers until I try to fix the bg color please :o

I will check tomorrow 💤😴

@XengShi
Copy link
Owner

XengShi commented Nov 20, 2024

Bruh It's not ready for review.

I accidentally touched that button 🙃 don't know how to ctrl+z

@prem-k-r
Copy link
Collaborator

I noticed one more bug. When 'Ai tools' is enabled and opened/expended. If you disable the Ai tools toggle shortcuts just disappear.

Hint : When disabling Ai tools toggle it should first close or collapse [AI tools] button

IMG_20241120_234851.png

The fold/unfold(little up/down arrow) botton might be affecting it.
I would say we should remove it, it's unnecessary.

@prem-k-r
Copy link
Collaborator

Also we have to see if mouse cursor is within resultBox, then keyboard selection must be unhighlighted

Rest of the things seems okay to me.

 if mouse cursor is within resultBox, then keyboard selection is unhighlighted
@prem-k-r
Copy link
Collaborator

if mouse cursor is within resultBox, then keyboard selection must be unhighlighted

okay, done this too.
@itz-rj-here please merge PR in your forked repo

Coloring in Search Suggestion when accessing through keyboard and some visual changes on resultBox
@itz-rj-here
Copy link
Collaborator Author

When the suggestions are hovered, and using the arrow key the arrow key lets the cursor go to the first and last point of the text.

image

@itz-rj-here
Copy link
Collaborator Author

Check the chrome search bar. Use the pointer to hover and use the arrow key to highlight. We need to setup it like that.

@itz-rj-here
Copy link
Collaborator Author

Main thing is we need to make it like 2 text will be highlighten when selecting a text from search suggestion.

@prem-k-r
Copy link
Collaborator

Shouldn't having 2 highlighted text. Have to choose one. Prioritising mouse cursor over keyboard.

@XengShi
Copy link
Owner

XengShi commented Nov 21, 2024

I think you should use a minimal symetric border-radius like Google does.

image
this?

The current one is looking better

@prem-k-r
Copy link
Collaborator

prem-k-r commented Nov 21, 2024

ok, then let's go?

btw how about decreasing gap between searchbar and suggestion box
image
or this much:
image

@itz-rj-here
Copy link
Collaborator Author

ok, then let's go?

btw how about decreasing gap between searchbar and suggestion box
image
or this much:
image

The upper one is more better

@itz-rj-here
Copy link
Collaborator Author

I think you should use a minimal symetric border-radius like Google does.

image
this?

The current one is looking better

I agree with you.

@prem-k-r
Copy link
Collaborator

ok, then let's go?
btw how about decreasing gap between searchbar and suggestion box
image
or this much:
image

The upper one is more better

ok
for it, delete break in html, and replace top: 1; with bottom: -100px; in css

@itz-rj-here
Copy link
Collaborator Author

ok, then let's go?
btw how about decreasing gap between searchbar and suggestion box
image
or this much:
image

The upper one is more better

ok for it, delete break in html, and replace top: 1; with bottom: -100px; in css

Which break?
I think we should go with the old css style. Its way better than the new one.

@prem-k-r
Copy link
Collaborator

ok for it, delete break in html, and replace top: 1; with bottom: -100px; in css

Which break? I think we should go with the old css style. Its way better than the new one.

almost same though
old:
image
image

new:
image
image

prem-k-r and others added 3 commits November 21, 2024 18:28
decreasing gap between searchbar and suggestion box
decreasing gap between searchbar and suggestion box
decreasing gap between searchbar and suggestion box
Co-authored-by: prem-k-r <60751338+prem-k-r@users.noreply.github.com>
@itz-rj-here itz-rj-here marked this pull request as ready for review November 21, 2024 13:35
@itz-rj-here
Copy link
Collaborator Author

itz-rj-here commented Nov 21, 2024

I think we should go with it.
@ceskyDJ @XengShi Can you check guys if you have any more suggestions about it?...

@itz-rj-here
Copy link
Collaborator Author

@ceskyDJ r u alive bruh 🗿?

@ceskyDJ
Copy link
Contributor

ceskyDJ commented Nov 22, 2024

@ceskyDJ r u alive bruh 🗿?

I'm sorry, I'm just too busy with school. I think it's not that bad. I'd personally append the suggestion list just under the search input and don't use so big border-radius for highlighting selected search suggestions, but overall, it's pretty usable.

@itz-rj-here
Copy link
Collaborator Author

@ceskyDJ r u alive bruh 🗿?

I'm sorry, I'm just too busy with school. I think it's not that bad. I'd personally append the suggestion list just under the search input and don't use so big border-radius for highlighting selected search suggestions, but overall, it's pretty usable.

@prem-k-r pls check

@XengShi
Copy link
Owner

XengShi commented Nov 22, 2024

I think we should go with it. @ceskyDJ @XengShi Can you check guys if you have any more suggestions about it?...

Working fine!

@XengShi XengShi merged commit a801377 into XengShi:main Nov 22, 2024
@prem-k-r
Copy link
Collaborator

If we align it with search text, it doesn't looks good, background Search with is partially visible, we have to hide it then when suggestions are visible.

For border-radius: 16px;, change it if needed, css line 153

@itz-rj-here
Copy link
Collaborator Author

If we align it with search text, it doesn't looks good, background Search with is partially visible, we have to hide it then when suggestions are visible.

For border-radius: 16px;, change it if needed, css line 153

No need I just came home and checked it one more time. It is looking way better than before.

@prem-k-r
Copy link
Collaborator

@ceskyDJ r u alive bruh 🗿?

I'm sorry, I'm just too busy with school. I think it's not that bad. I'd personally append the suggestion list just under the search input and don't use so big border-radius for highlighting selected search suggestions, but overall, it's pretty usable.

image

@prem-k-r
Copy link
Collaborator

image
shifting doesn't looks good to me, but we can change radius to this, if you all agree

@prem-k-r
Copy link
Collaborator

image shifting doesn't looks good to me, but we can change radius to this, if you all agree

I like this one more,
@itz-rj-here @XengShi, you?

@itz-rj-here
Copy link
Collaborator Author

image shifting doesn't looks good to me, but we can change radius to this, if you all agree

I like this one more,
@itz-rj-here @XengShi, you?

Okayee do that.

@XengShi
Copy link
Owner

XengShi commented Nov 22, 2024

Yes, search suggestions width = searchbar's width is looking better

@prem-k-r
Copy link
Collaborator

yeah, but what about border radius, @ceskyDJ asked for smaller radius
this one?
image

or what we already have?
image

@XengShi
Copy link
Owner

XengShi commented Nov 22, 2024

Just use one which matches perfectly with other elements of extension.

@prem-k-r
Copy link
Collaborator

1
image

or 2
image

@itz-rj-here
Copy link
Collaborator Author

1
image

or 2
image

Actually 1 😅

@prem-k-r
Copy link
Collaborator

Ok

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.

Add ability to select items from search autocomplete via keyboard
4 participants