-
Notifications
You must be signed in to change notification settings - Fork 3k
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.
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
[HOLD for payment 2022-11-02] [$250] Saving a workspace name should close the keyboard - reported by @chauchausoup #11195
Comments
Triggered auto assignment to @NicMendonca ( |
ProposalAdd Keyboard.dismiss() after line 64 (not sure if we should dismiss the keyboard before validation or after, do we wanna keep the keyboard open if there is some error?) App/src/pages/workspace/WorkspaceSettingsPage.js Lines 61 to 64 in f76f708
We have to do the same for other pages too because it's not only on workspace settings page. |
Triggered auto assignment to @Luke9389 ( |
@Puneet-here please wait for the Help Wanted label to be applied before leaving a proposal. Thanks! |
Triggered auto assignment to @MitchExpensify ( |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @rushatgabhane ( |
Current assignee @Luke9389 is eligible for the External assigner, not assigning anyone new. |
Sorry for proposing early, I like the idea of waiting for the label to be applied. But recently most of the contributors are proposing just after the issue gets created. Because of it we really don't get the chance to propose a solution if we wait for the label. |
It would be good to add something like below in contributing guideline:
This way, contributors save time wasting to search for non cc: @mallenexpensify (sorry but I am tagging you because you were the last man to update CONTRIBUTING.md) |
Thanks so much for the feedback! |
ProposalI got confused after clicking the save button because there's no feedback and closing the keyboard doesn't make me realize if the save is a success. I'm suggesting navigating the page back to the workspace initial page after submitting or adding a growl notification. diff --git a/src/pages/workspace/WorkspaceSettingsPage.js b/src/pages/workspace/WorkspaceSettingsPage.js
index 40c370abb..ad3503a18 100644
--- a/src/pages/workspace/WorkspaceSettingsPage.js
+++ b/src/pages/workspace/WorkspaceSettingsPage.js
@@ -23,6 +23,8 @@ import withPolicy, {policyPropTypes, policyDefaultProps} from './withPolicy';
import {withNetwork} from '../../components/OnyxProvider';
import OfflineWithFeedback from '../../components/OfflineWithFeedback';
import FullPageNotFoundView from '../../components/BlockingViews/FullPageNotFoundView';
+import Navigation from '../../libs/Navigation/Navigation';
+import ROUTES from '../../ROUTES';
const propTypes = {
...policyPropTypes,
@@ -65,6 +67,7 @@ class WorkspaceSettingsPage extends React.Component {
const name = this.state.name.trim();
const outputCurrency = this.state.currency;
Policy.updateGeneralSettings(this.props.policy.id, name, outputCurrency);
+ Navigation.navigate(ROUTES.getWorkspaceInitialRoute(this.props.policy.id));
}
validate() { Result Screen.Recording.2022-09-25.at.00.13.46.mov |
Looks like something related to As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our Feel free to drop a note in #expensify-open-source with any questions. |
|
Don't agree with the expected results because https://github.com/Expensify/App/blob/main/contributingGuides/FORMS.md#form-submission @Luke9389 How about we refactor this page to use Form.js? It should fix this issue and also get a refactor done |
Proposal App/src/pages/workspace/WorkspaceSettingsPage.js Lines 61 to 64 in f76f708
To disable button we need to add isDisabled={this.state.isDisabled ? true : false} in the button props We need to add a new state isDisabled. this.state = { App/src/pages/workspace/WorkspaceSettingsPage.js Lines 40 to 43 in f76f708
We need to change state of isDisabled on Change of state of all fields in WorkspaceSettingPage to enable button again. onImageSelected={(file) => {
}} onInputChange={(currency) => {
}} onChangeText={(name) => {
}} submit Function should look something like this by adding Keyboard.dismiss() and setting this.setState({isDisabled: true}). submit() {
} Simulator.Screen.Recording.-.iPhone.13.Pro.-.2022-09-27.at.02.02.50.mp4 |
@rushatgabhane I like the approach of refactoring to use Form.js 👍 |
📣 @mollfpr You have been assigned to this job by @Luke9389! |
My bad @mollfpr, didn't realize I needed to assign you! |
@chauchausoup mind applying for the Upwork job for eventual reporting bonus? |
@mollfpr is working on a PR. Not overdue. |
@MitchExpensify can't open the Upwork link. |
@chauchausoup this should work for ya https://www.upwork.com/jobs/~014b51c853743c4959 |
Hired @chauchausoup, thanks for sharing the working link @mallenexpensify ! |
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.19-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2022-11-02. 🎊 |
Made a note to pay on 2022-11-02 |
Paid to all bar @mollfpr, just waiting on you to accept the offer! Thanks everyone |
@MitchExpensify accepted, thanks! |
Paid all, thanks again |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Action Performed:
Expected Result:
Keyboard modal should close
Save button should be frozen as in web app
Actual Result:
Nothing happens. It remains as it is once a desired name is updated.
Workaround:
unknown
Platform:
Where is this issue occurring?
Version Number: 1.2.1-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Screen.Recording.2022-09-04.at.11.52.02.mov
Expensify/Expensify Issue URL:
Issue reported by: @chauchausoup
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1662272007043879
View all open jobs on GitHub
The text was updated successfully, but these errors were encountered: