Closed marcaaron closed 2 days ago
Triggered auto assignment to @twisterdotcom (NewFeature
), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.
:warning: It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time :warning:
Job added to Upwork: https://www.upwork.com/jobs/~01644d92b58b119eb5
Triggered auto assignment to Contributor-plus team member for initial proposal review - @eh2077 (External
)
@nkdengineer Removing your proposal as it's insufficient. Please post a full proposal once you are done drafting it.
onClearInput
prop to TextInput
that will trigger once user clicks on it. We should add it to the TextInput component and we can use entire the app. To do this, in here, add:
{isFocused && !isReadOnly && shouldShowClearButton && value && (
<Tooltip text="Clear">
<PressableWithoutFeedback
style={[styles.mt4, styles.ml1]}
onMouseDown={(e) => {
e.preventDefault();
}}
onPress={(e) => {
onInputChange?.(''); // Can use setValue('') as well
onClearInput?.(inputID);
}}
>
<ClearIcon /> // Based on design
</PressableWithoutFeedback>
</Tooltip>
)}
isFocused && !isReadOnly && shouldShowClearButton && value: true
TextInput
in FormWrapper
rather than use TextInput
individually, so in FormProvider we should create a new function (do not need now, but in future), named onClearInput
:
onClearInput: (key) => {
// Side effect when clearing input
},
<FormProvider>
<InputWrapper InputComponent={TextInput} shouldShowClearButton />
</FormProvider>
Improve UX of modifying single line text inputs populated with very long text
New feature
To fix this issue we can update BaseTextInput
and add new component which will have the same styles as the loader
Draft button ( It is a little unclear when we should show the button (Immediately after entering the first character or at a certain length of text?)
Following this comment I think we'd better disable this button for mobile devices https://github.com/Expensify/App/issues/40313#issuecomment-2063175554
<PressableWithoutFeedback
style={[styles.mt4, styles.ml1]}
onPress={() => {
setValue('');
input.current?.focus();
}}
>
<Icon
src={Expensicons.Close}
fill={theme.icon}
/>
</PressableWithoutFeedback>
I don't remember that we have a round icon with x But we can make something similar using styles
And in case Group name we can update group name screen to show error when we clear input and try to save Because in this case we just use previous value
Plus we need to add new param to make this optional But I think it's not a bad idea to make this button for all inputs Because We have many places where it would be convenient to delete all entered text
But for this we need to add condition to show element only when isLoading is false (But here it already depends on the design team )
NA
@eh2077 I updated proposal
@marcaaron I doubt we should add such icon to help clear long text. I see following downsides if adding the clearing icon
I tested deleting text on both Android (using emulator) and iOS devices and I found that we only need to long press the text, press Select all
and then press delete. I don't see it's that difficult to delete them, see below clips
iOS | Android |
---|---|
Adding an x
circle icon to allow clearing the input when updating an existing Group chat "name" field
New feature
To address the issue of improving the user experience when modifying single line text inputs populated with very long text, we propose the following changes:
x
button to allow users to clear the input field. This button should be styled consistently with other UI elements and added within the BaseTextInput
component.{isFocused && !isReadOnly && shouldShowClearButton && value && (
<Tooltip text="Clear">
<PressableWithoutFeedback
style={[styles.mt4, styles.ml1]}
onMouseDown={(e) => {
e.preventDefault();
}}
onPress={(e) => {
onInputChange?.('');
onClearInput?.(inputID);
}}
>
<ClearIcon />
</PressableWithoutFeedback>
</Tooltip>
)}
onClearInput
prop to trigger the clearing functionality when the user interacts with the clear button. Ensure this logic is incorporated into the BaseTextInput
component.
onClearInput: (key) => {
// Implement side effects when clearing input
// For example:
setValue('');
},
{isFocused && !isReadOnly && shouldShowClearButton && value && (
// Render the clear button
)}
N/A
@twisterdotcom What do you think of this https://github.com/Expensify/App/issues/40313#issuecomment-2063175554 ? Is it really necessary to add this feature?
cc @marcaaron
Thank you all for your proposals.
I think we can go with @nkdengineer 's proposal if we want to add this feature. I also added my thoughts about this feature here https://github.com/Expensify/App/issues/40313#issuecomment-2063175554.
@nkdengineer was first to figure out the key ideas for implementing this feature, and their refined version includes additional details that need consideration. However, it's advisable to avoid excessive editing to maintain clarity.
🎀👀🎀 C+ reviewed
Current assignee @marcaaron is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.
@eh2077 While I appreciate the concerns I don't share them. This is a fairly common pattern.
The problem with the current design is that the field will always be very full of text if you invite a few people or more so it's an improvement over long press, select all, delete (which is harder to perform).
📣 @eh2077 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!
📣 @nkdengineer 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!
Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑💻 Keep in mind: Code of Conduct | Contributing 📖
@marcaaron Can you give me a design for the "Clear" icon? Thanks
Triggered auto assignment to @dannymcclain (Design
), see these Stack Overflow questions for more details.
Here is the x-circle
icon: x-circle.svg.zip
I thought we already had it in App/assets/images
but I was wrong!
@dannymcclain Please finalize the size of the button and its position. Below are the results when I set the height and width both to 18. | default | center |
---|---|---|
We generally do base4 for all of our sizing, so I have a feeling this should be 20x20. Will wait for Danny to chime in though!
Yeah let's stick with 20x20! I'm fine with the default image you've shown there.
Looks like this was deployed to prod 5 days ago, so I need to pay @eh2077 and @nkdengineer in a few days right? Not sure why the automation didn't work.
Coming from https://expensify.slack.com/archives/C05RECHFBEW/p1712957589044869
Problem
When updating an existing Group chat "name" field in the App it is difficult to clear the pre-populated text in mobile views and on mobile devices.
Example:
Solution
Let's solve this problem by adding an x circle icon to allow clearing the input.
Let's also do this in a cross-platform way and universally across different areas of the app where it might be useful.
Deliverables:
TextInput
Upwork Automation - Do Not Edit