Skip to content

fix: correct input focus for multiple components - #91

Open
bbjbc wants to merge 1 commit into
Wondermarin:mainfrom
bbjbc:fix/correct-input-focus-90
Open

fix: correct input focus for multiple components#91
bbjbc wants to merge 1 commit into
Wondermarin:mainfrom
bbjbc:fix/correct-input-focus-90

Conversation

@bbjbc

@bbjbc bbjbc commented Sep 11, 2025

Copy link
Copy Markdown

Hello @Wondermarin. Thank you for this awesome project!

Description of Change

This PR fixes a bug(#90) where clicking a label on a second color picker component would incorrectly focus the input field of the first one.

The root cause was that the id attributes for the inputs were hardcoded, causing duplicate IDs when multiple components were rendered.

I resolved this by using the useId hook to generate a unique ID for each component instance. This ensures that every <label>'s htmlFor attribute correctly points to its corresponding <input>, making the component work reliably when used multiple times on the same page.

@Wondermarin Wondermarin left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

First of all, I want to thank you for your contribution! However, there are a few issues with the proposed changes:

  1. We can鈥檛 use useId because we aim to support React starting from version 16.8, while the hook you used was introduced only in version 18. Therefore, we need to implement a client-side ID generation mechanism without relying on any external libraries (as we must remain dependency-free).
  2. If we implement client-side ID generation, we also need to handle SSR environments where we must ensure consistent IDs on both the client and the server to avoid hydration issues. The only solution I see is to require SSR environments to pass a stable ID as a prop, which will be used on both client and server. Otherwise, we would fall back to a client-generated ID and log a console warning that this may cause hydration problems.

These two issues need to be resolved before I can merge the branches. I鈥檝e had some rough drafts before, but currently, I don鈥檛 have enough time to polish and finalize the changes. You can take this on, or close the PR so I can address it when I get back home.

@bbjbc

bbjbc commented Sep 18, 2025

Copy link
Copy Markdown
Author

Hi! @Wondermarin. Thanks for the detailed feedback. I have implemented a React 16.8+ compatible solution to replace useId.
I created a custom useUniqueId hook.

import { useRef } from "react";

let idCounter = 0;

export const useUniqueId = (): string => {
  const idRef = useRef<string>();

  if (idRef.current === undefined) {
    idCounter += 1;
    idRef.current = `rcp-${idCounter}`;
  }

  return idRef.current;
};

Above that ensures each component gets a unique ID and maintains the same ID across re-renders using a simple counter approach for clinet-side generation.
Does this approach work for your requirements?

Also curious! what's driving the decision to maintain React 16.8+ compatibility instead of bumping to React 18+? Is it mainly for supporting legacy projects, or are there other factors? Would help me understand the project's constraints better.

Thank you !

@mickeys000

Copy link
Copy Markdown

Hi both. I was wondering if this PR is still a WIP?

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