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.

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

Edit workspace page gets automatically closed on Submit #4491

Closed
aman-atg opened this issue Aug 7, 2021 · 40 comments
Closed

Edit workspace page gets automatically closed on Submit #4491

aman-atg opened this issue Aug 7, 2021 · 40 comments
Assignees
Labels
External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@aman-atg
Copy link
Contributor

aman-atg commented Aug 7, 2021

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:

  1. Login to newDot
  2. Go to Workspace page
  3. Edit Workspace name
  4. Save and it'll close itself automatically

Expected Result:

Shouldn't close itself and additionally we can show a feedback message instead
(it should behave like Edit Profile Page for consistency)

Actual Result:

expensifyBug-2021-08-07_12.16.56.mp4
bug2.mp4

Workaround:

Open the edit page again.

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Version Number:
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:

View all open jobs on Upwork

@aman-atg aman-atg added AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 labels Aug 7, 2021
@MelvinBot
Copy link

Triggered auto assignment to @zanyrenney (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

@MelvinBot MelvinBot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Aug 7, 2021
@aman-atg
Copy link
Contributor Author

aman-atg commented Aug 7, 2021

Proposal

  • We can fix this by removing Navigation.dismissModal() from line 245

    function update(policyID, values) {
    API.UpdatePolicy({policyID, value: JSON.stringify(values), lastModified: null})
    .then((policyResponse) => {
    if (policyResponse.jsonCode !== 200) {
    // Show the user feedback
    const errorMessage = translateLocal('workspace.editor.genericFailureMessage');
    Growl.error(errorMessage, 5000);
    return;
    }
    Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, values);
    Navigation.dismissModal();
    });
    }

  • Additionally, we can show a Growl here

@mananjadhav
Copy link
Collaborator

Possible duplicate of #4487 ?

@aman-atg
Copy link
Contributor Author

aman-atg commented Aug 7, 2021

That one is related to "Save" Button and this one is about the overall workflow, Right?

@mananjadhav
Copy link
Collaborator

For me, the sidebar closing is the expected behavior. Because once you've edited you'd want to go back to the Workspace page, why another additional click?

But I agree with the feedback point that you've mentioned, and that's what I was pointing out with #4487

@aman-atg
Copy link
Contributor Author

aman-atg commented Aug 7, 2021

For me, the sidebar closing is the expected behavior. Because once you've edited you'd want to go back to the Workspace page, why another additional click?

After saving the name, You might want to edit profile picture too but that won't be possible.

And We should show consistent behaviour. We're not closing Edit Profile after saving then why this one.

@parasharrajat
Copy link
Member

Hey guys, @aman-atg @mananjadhav I think the Edit Workspace feature is just landed in the APP and internal QA is going on #4447. Thus all the issues that are coming from this will be linked to the original PR.

@MelvinBot
Copy link

Triggered auto assignment to @Beamanator (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@MelvinBot MelvinBot removed the Overdue label Aug 9, 2021
@zanyrenney zanyrenney removed their assignment Aug 9, 2021
@Beamanator
Copy link
Contributor

@aman-atg Thanks for reporting something you think should change, but please try to do some research to determine if this was the desired functionality or not (as you can see in this PR's testing steps, this is the desired functionality). If you disagree with a previous decision / if you think some functionality should change, please bring that up in #expensify-open-source so it can get many 👀 and can be discussed by multiple people.

Here's the exact test steps I am referring to from the source PR:

  1. In that modal, change everything and press "Save"
  2. The modal should close, and everything you changed in the modal should get updated everywhere.

cc @HorusGoul

@Beamanator
Copy link
Contributor

I'm going to close this for now, but if you still feel like it needs to be changed please bring it up in the public channel and we can re-open this if necessary 👍

@aman-atg
Copy link
Contributor Author

aman-atg commented Aug 9, 2021

Thanks @Beamanator, I'll try to not miss such things from next time.

But yes I still feel this might affect the UX.
So, here's the slack thread for this one. (thread)

@aman-atg
Copy link
Contributor Author

aman-atg commented Aug 9, 2021

@Beamanator
Ah. I guess it's not possible for external contributors to re-open an issue. ( Slack )

@Beamanator Beamanator reopened this Aug 10, 2021
@Beamanator Beamanator changed the title Edit workspace page gets automatically closed on Submit [HOLD] Edit workspace page gets automatically closed on Submit Aug 10, 2021
@Beamanator Beamanator added the Planning Changes still in the thought process label Aug 10, 2021
@Beamanator
Copy link
Contributor

Yeah I think we're good, sorry for the delay - let's go with Aman's suggestion: Your workspace was successfully saved

@mallenexpensify please hire @aman-atg for this job :D And @aman-atg please submit a PR when you have the chance!

@mallenexpensify
Copy link
Contributor

Job posted - https://www.upwork.com/jobs/~01127dbb90e28a1001
@aman-atg please submit a proposal there and confirm here in a comment when you have, thanks.

@aman-atg
Copy link
Contributor Author

Submitted the proposal.

@aman-atg aman-atg mentioned this issue Sep 15, 2021
5 tasks
@mallenexpensify
Copy link
Contributor

Hired in Upwork!

@kadiealexander
Copy link
Contributor

Hi @aman-atg, I've placed this issue on hold as per this update, as we are prioritising issues related to a feature release scheduled for 9/24. As an apology for the delay, we will add a $100 bonus as a thank you for waiting.

@mallenexpensify
Copy link
Contributor

On N6 hold, coming off soon

@Beamanator
Copy link
Contributor

Still on hold

@MelvinBot MelvinBot removed the Overdue label Oct 5, 2021
@kadiealexander
Copy link
Contributor

Please refer to this post for updated information on the n6 hold, we've raised the bonus to $250 for all issues where a PR is created before the N6 hold is lifted.

@Beamanator Beamanator changed the title Edit workspace page gets automatically closed on Submit [HOLD] Edit workspace page gets automatically closed on Submit Oct 11, 2021
@mallenexpensify
Copy link
Contributor

Not overdue

@mallenexpensify
Copy link
Contributor

n6-hold is lifted, label removed!
@Beamanator can you review @aman-atg 's PR plz #5269

@mallenexpensify mallenexpensify changed the title [HOLD] Edit workspace page gets automatically closed on Submit Edit workspace page gets automatically closed on Submit Oct 20, 2021
@Beamanator
Copy link
Contributor

Yes will do now!

@Beamanator
Copy link
Contributor

Actually we're asking everyone to merge main into their branches since we've had lots of updates since the beginning of the n6-hold. @aman-atg when you get the chance, will you please merge main into your branch (#5269) and test to make sure everything is still working well, then ping me in your PR?

@Beamanator
Copy link
Contributor

@mallenexpensify as noted here, this change was made in this PR during the n6-hold so I closed the PR but let's still pay out @aman-atg 👍

@mallenexpensify
Copy link
Contributor

@Beamanator should I pay @aman-atg for the solution/fix as well as reporting?

@Beamanator
Copy link
Contributor

Yeah, it looks like that's the case, thanks @mallenexpensify 🙏

@mallenexpensify
Copy link
Contributor

mallenexpensify commented Nov 2, 2021

Paid @aman-atg with bonus in Upwork, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
None yet
Development

No branches or pull requests

9 participants