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.

Sign up for GitHub

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

Rename TextInputFocusable to Composer ⌨️ & add Stories For TextInputs and fix bugs #7984

Merged
merged 17 commits into from
Mar 11, 2022
Merged

Rename TextInputFocusable to Composer ⌨️ & add Stories For TextInputs and fix bugs #7984

Changes from 2 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
5aaced4
Rename TextInputFocusable to Composer
parasharrajat Mar 1, 2022
cd2f82c
Add TextInput Stories
parasharrajat Mar 2, 2022
b8be9ce
fix param type for story
parasharrajat Mar 2, 2022
3efd5e2
Story for Composer
parasharrajat Mar 2, 2022
f9ad098
Fix: autogrow Story
parasharrajat Mar 7, 2022
4955180
Move value to state to trigger updates on value change
parasharrajat Mar 7, 2022
6c1c388
Fix: AutoGrow Input styling
parasharrajat Mar 7, 2022
a7407a6
Visually Hide internal Text
parasharrajat Mar 7, 2022
ab066cb
Add comments
parasharrajat Mar 7, 2022
564b55c
Show the placeholder when forceActiveLabel is true
parasharrajat Mar 7, 2022
301e605
Merge branch 'main' of github.com:Expensify/Expensify.cash into texti…
parasharrajat Mar 9, 2022
e70019e
refactor and rename
parasharrajat Mar 9, 2022
ec717fe
Clean up and refactor
parasharrajat Mar 10, 2022
6c2f5c7
Change error message
parasharrajat Mar 11, 2022
8a7d658
clean up Stories
parasharrajat Mar 11, 2022
b066c8d
fix labels
parasharrajat Mar 11, 2022
ee1ecba
Update stories
parasharrajat Mar 11, 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
49 changes: 18 additions & 31 deletions src/stories/TextInput.stories.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ ErrorStory.args = {
const ForceActiveLabel = Template.bind({});
ForceActiveLabel.args = {
label: 'Forced active label',
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
placeholder: 'My placeholder text',
forceActiveLabel: true,
};

Expand All @@ -57,46 +58,34 @@ Placeholder.args = {
placeholder: 'My placeholder text',
};

const AutoGrow = Template.bind({});
AutoGrow.storyName = 'Autogrow input';
AutoGrow.args = {
const AutoGrowInput = Template.bind({});
AutoGrowInput.args = {
label: 'Autogrow input',
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's standardize the case-convention for the labels: AutoGrow Input since the other labels seem to capitalize everything.

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems older ones have first as only capital letter so I am going with that following the page titles in the app.

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like there are still two main conventions for labels:

  1. Input with xyz - e.g. Input with default value
  2. xyz input - e.g. Prefixed input

I'm fine with either options but i do think we should pick one

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated most of them. Left few whose name makes more sense. But I would say it does not matter much. The main perspective should be that they should clearly show the proper example of that component/varient

name: 'AutoGrow',
placeholder: 'My placeholder',
placeholder: 'My placeholder text',
autoGrow: true,
textInputContainerStyles: [{
minWidth: 150,
}],
};

const Prefixed = Template.bind({});
Prefixed.storyName = 'Prefixed input';
Prefixed.args = {
const PrefixedInput = Template.bind({});
PrefixedInput.args = {
label: 'Prefixed input',
name: 'Prefixed',
placeholder: 'My placeholder text',
prefixCharacter: '@',
};

const WithFixedLabel = Template.bind({});
WithFixedLabel.storyName = 'Always active label';
WithFixedLabel.args = {
label: 'Active label input',
name: 'activelabel',
placeholder: 'My placeholder text',
forceActiveLabel: true,
};

const MaxLength = Template.bind({});
MaxLength.storyName = 'Input with maxLength';
MaxLength.args = {
label: 'Label',
name: 'inputmaxlength',
const MaxLengthInput = Template.bind({});
MaxLengthInput.args = {
label: 'MaxLength input',
name: 'MaxLengthInput',
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
placeholder: 'My placeholder text',
maxLength: 50,
};

const HintErrorInput = (args) => {
const HintAndErrorInput = (args) => {
const [error, setError] = useState('');
return (
<TextInput
Expand All @@ -113,10 +102,9 @@ const HintErrorInput = (args) => {
/>
);
};
HintErrorInput.storyName = 'Input with hint & error';
HintErrorInput.args = {
label: 'Label',
name: 'inputhint&error',
HintAndErrorInput.args = {
label: 'HintAndError input',
name: 'HintAndErrorInput',
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
placeholder: 'My placeholder text',
hint: 'Type "Oops!" to see the error.',
};
Expand All @@ -129,9 +117,8 @@ export {
ErrorStory,
ForceActiveLabel,
Placeholder,
AutoGrow,
Prefixed,
WithFixedLabel,
MaxLength,
HintErrorInput,
AutoGrowInput,
PrefixedInput,
MaxLengthInput,
HintAndErrorInput,
};