Skip to content

Conversation

@Venki1402
Copy link

@Venki1402 Venki1402 commented Aug 11, 2025

Screenshot 2025-08-11 at 11 51 07 AM

Copy link
Member

@adamant-pwn adamant-pwn left a comment

Choose a reason for hiding this comment

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

Hi, thanks for the pull request! It generally looks good, but I'd like to clarify some things first (see comments).

}

/* Only show on wider viewports where the inline search is visible */
@media (max-width: 959px) {
Copy link
Member

Choose a reason for hiding this comment

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

Where is this value defined? Can we somehow take it from the theme named constants?

Copy link
Author

Choose a reason for hiding this comment

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

not sure regd. that, can we take 60em instead?

Comment on lines +88 to +96
justify-content: center;
height: 20px;
padding: 0 6px;
border-radius: 4px;
border: 1px solid var(--md-default-fg-color--lightest, rgba(0,0,0,.2));
background: var(--md-default-bg-color, #fff);
color: var(--md-default-fg-color, rgba(0,0,0,.87));
font: 500 12px/1 var(--md-text-font, system-ui, -apple-system, Segoe UI, Roboto, Ubuntu, Cantarell, Noto Sans, sans-serif);
opacity: .72;
Copy link
Member

Choose a reason for hiding this comment

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

These seem to mostly repeat things that are already set in .kbd-hint. I'm not that well versed in CSS, is this really necessary? Why do we need them as two independent settings?

Copy link
Author

Choose a reason for hiding this comment

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

they’re separate because one is a DOM element and the other is a pseudo-element positioned inside the input. If we want both contexts covered (inside the field on desktop, next to the icon on mobile/when the field is hidden), we need both targets.

Copy link
Member

Choose a reason for hiding this comment

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

What do you mean by next to the icon on mobile / when the field is hidden? For me, when the field is hidden, the hint is hidden as well?

image

@mhayter
Copy link
Contributor

mhayter commented Sep 27, 2025

Any update @Venki1402 ?

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.

feat (enhancement) : on event cmd + k -> focus on search bar

3 participants