Skip to content
This repository was archived by the owner on Nov 3, 2024. It is now read-only.

Feat: Logic to show placeholder on inputs conditionally. #46

Closed
wants to merge 2 commits into from
Closed

Feat: Logic to show placeholder on inputs conditionally. #46

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3db2c44
Add placeholder option for input
franmc01 May 24, 2022
0492dda
Conditional placeholder added
franmc01 May 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ type Props = {
containerClassName?: string;
inputClassName?: string;
isPassword?: boolean;
placeholderCharacter?: string & { length: 1 };
showPlaceholder: false;
Copy link
Owner

Choose a reason for hiding this comment

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

I think we can handle the display of the placeholder using the placeholderCharacter prop, or do you see any case where we pass it and don't want to display it? Also, I would rename the prop to only placeholder

Copy link
Author

Choose a reason for hiding this comment

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

Hello @drac94, if I tell you the idea of ​​managing the "show Placeholder" variable to hide and show the placeholder is born from the need of the client, some companies manage that their input forms have a mask while others prefer that they remain without a mask , so the idea is that the package allows to do that configuration in a quite simple way, I don't know if I made myself understood hehe

Copy link
Author

Choose a reason for hiding this comment

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

Regarding the name of the variable, if it is true, it is a better placeholder, the one that locates it is too verbose

Copy link
Owner

Choose a reason for hiding this comment

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

I think I understand the motivation for this feature and I can see how it is valuable, but in terms of implementation, we can just have a placeholder prop and nothing else, if it is set, we show the placeholder otherwise we don't. Basically this:

type Props = {
  allowedCharacters?: typeof allowedCharactersValues[number];
  ...
  placeholder?: string & { length: 1 };
}

...

<input
  ...
  placeholder={placeholder}
/>

Copy link
Author

Choose a reason for hiding this comment

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

It's perfect!!

onChange: (res: string) => void;
};

Expand Down Expand Up @@ -68,6 +70,8 @@ const AuthCode = forwardRef<AuthCodeRef, Props>(
length = 6,
containerClassName,
inputClassName,
showPlaceholder,
placeholderCharacter = '0',
isPassword = false,
onChange
},
Expand Down Expand Up @@ -196,6 +200,7 @@ const AuthCode = forwardRef<AuthCodeRef, Props>(
inputsRef.current[i] = el;
}}
maxLength={1}
placeholder={showPlaceholder ? placeholderCharacter : ' '}
className={inputClassName}
autoComplete={i === 0 ? 'one-time-code' : 'off'}
aria-label={
Expand Down