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

Unsaved content warning not appearing for logged in 'New Project' flow #1110

Closed
vikasrohit opened this issue Aug 10, 2017 · 10 comments
Closed
Assignees
Labels

Comments

@vikasrohit
Copy link

vikasrohit commented Aug 10, 2017

When user open the New project modal by using the 'New project' button from the topbar after logging in and closes the modal or refreshes the page after reaching the project details page, it does not show the Unsaved content warning which we show when the same flow is done using /new-project URL.

fyi, @fnisen

@vikasrohit vikasrohit self-assigned this Aug 10, 2017
@fnisen
Copy link
Contributor

fnisen commented Aug 11, 2017

Can you please give more detail on this one?

@fnisen fnisen closed this as completed Aug 11, 2017
@fnisen fnisen reopened this Aug 11, 2017
@vikasrohit
Copy link
Author

@fnisen as per our discussion in #1037 (comment) we have removed the unsaved content warning from /new-project flow (it doesn't matter if user is logged in or logged out). Now I am wondering how we should handle it when user is logged in and wants to create project from 'New Project' button, I think we are not saving incomplete project in this case, hence, we need to show the unsaved content warning here, right?

@fnisen fnisen added the P2 label Sep 7, 2017
@fnisen
Copy link
Contributor

fnisen commented Sep 7, 2017

@vikasrohit yes, I see. Good point. In this case, let's show the warning b/c the project is not being saved in local storage, same as we had been doing if you closed the tab before saving changes on the Specifications page.

@vikasrohit
Copy link
Author

@fnisen I have implemented the functionality, however, I would like to get it tested before merging into dev. And right now test01 env is being used for penetration testing, so I am not sure how can I deploy it for your testing without merging into dev.

@fnisen
Copy link
Contributor

fnisen commented Sep 8, 2017

Sure. Do you want to wait until test01 is clear and then use it?

@vikasrohit
Copy link
Author

I am inclining towards deploying it to dev because test01 is going to be freed at least 2 weeks later. We have to just make sure that we don't deploy the dev changes to production, until we find this change stable. More or less it is stable, I just want to make sure of edge cases.

@vikasrohit
Copy link
Author

Deploying to dev. Please let me know if you face any blocker.

@fnisen
Copy link
Contributor

fnisen commented Sep 13, 2017

@vikasrohit one of the edge cases is the "Back" action. In that case we don't seem to be wiping the data; I can select another project and the data is still there. However, once I click "Back," if I start another project and then close it, I don't get the warning. Maybe just show the warning and wipe the data on "Back," as well, to be consistent?

@mtwomey
Copy link
Contributor

mtwomey commented Sep 25, 2017

@vikasrohit Let's go ahead and deploy please. Don't worry about the penn-testing. Kindly advise if you have and concerns.

@vic-tian
Copy link
Contributor

vic-tian commented Oct 3, 2017

@vikasrohit is this released? please update the status

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants