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

Conversation

franmc01
Copy link

The goal of this pull request is mostly to support what a dynamic placeholder is. By default it would be '0' in my implementation, but of course you can choose another character by default.

One of my motivations was what @hailwood mentioned
in issue #43

@@ -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!!

@drac94
Copy link
Owner

drac94 commented Jun 2, 2022

Hi @francmarin98, thanks for submitting the PR, I left a comment there

@drac94
Copy link
Owner

drac94 commented Jun 7, 2022

Changes applied in #49

@drac94 drac94 closed this Jun 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants