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

migrate to labels for search fields #262

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

florence-wolfe
Copy link
Collaborator

We were previously manually querying for elements and controlling their focus state.

instead we'll use label elements and let that element handle it as intended.

some ids are changed in this PR to try and ensure no conflicts if there are more than 1 search on the same page. we'll need to be more exhaustive about this in the future. maybe a lint rule could catch this.

zoglo
zoglo previously approved these changes Jun 24, 2024
Copy link
Collaborator

@zoglo zoglo left a comment

Choose a reason for hiding this comment

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

We may want to re-add the cursor pointer since it's gone due to label having higher specificity than the class in their parent-element.

https://github.com/therungg/therun-frontend/blob/db0a8c5fd1ae43a2e1d887bb5fb56627be40818c/src/styles/_globals.scss#L23-#L27

If you change this to the following code, it should work:

.input-group,
.form-select,
.cursor-pointer,
label.input-group-text {
  cursor: pointer;
}

src/components/game/category-leaderboards.tsx Show resolved Hide resolved
src/components/game/game-leaderboards.tsx Show resolved Hide resolved
@florence-wolfe florence-wolfe marked this pull request as draft June 24, 2024 21:24
# Conflicts:
#	src/components/search/autocompletion.tsx
@zoglo zoglo added a11y Accessibility features UX/UI Issue involves UX/UI labels Jul 10, 2024
@zoglo zoglo marked this pull request as ready for review July 10, 2024 00:55
@zoglo zoglo self-requested a review July 10, 2024 00:56
@zoglo zoglo requested a review from therungg July 10, 2024 00:58
@zoglo zoglo marked this pull request as draft July 10, 2024 00:59
@zoglo
Copy link
Collaborator

zoglo commented Jul 10, 2024

Converting to a draft due to duplicate ids.
@flo - just mark it as ready when this is resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y Accessibility features UX/UI Issue involves UX/UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants