mozilla
Fixing #2479: Allow user to change the project description without needing to publish their project.
Description wasn't saving if the project wasn't published. Now, by clicking cancel while the project is unpublished, the description is saved. Please review this and let me know if there's something that can be fixed.
OK - thanks for submitting this PR @jmrodriguesgoncalves!
My suggestion for this is to save the description when someone clicks outside of the Publish dialog and the dialog closes it. Currently, if I change the description and then click outside of the popup dialog, the change is lost. Hitting the Cancel button doesn't feel like it should save the description, which it does currently. So, in summary I'd recommend...
- Save the new description if someone clicks outside of the publish dialog
- Throw away the description of someone clicks the
Cancelbutton
Let me know your thoughts!
I think that sounds good, I can try to do what you are asking me. Clicking outside would save the description as you said, so we'd use the same code we have now, except, when clicking anywhere outside the Publish dialog.
Now, if we click the cancel button, should the description be completely erased, as in, making it blank, or should we have it be the way it was before, which basically kept the string intact while editing the project (so one would still see it there if they were to open the publish dialog again), but not saved if not published (so it would not show up when looking at their list of projects, or if closed and opened again for editing)?
Yeah the Cancel case is a bit strange for us. I just thought of a great solution however...
What if we just remove the Cancel button from the UI altogether? That way, you HAVE to click outside of the popup to close it, at which point we'll just save whatever changes you made.
The other little popups work this way already, the settings panel and file dropdown for example.
What do you think @jmrodriguesgoncalves ? Simplifies things a bit for us.
@flukeout I think that is a great idea that not only would simplify the code, but would also be much less confusing for the end user.
If we are to go ahead with this idea, I have a quick question for you since you have more familiarity with this code. Just like we talked about on the chat, I am struggling to find the function that deals with the click outside of the dialog box. I have tried setting an event listener for the div under it, "#click-underlay", however that does not seem to work.
If you have any input and if we are to go ahead with the UI change as well, let me know and I will get working on it. Thanks again!
Hey @jmrodriguesgoncalves - yeah let's def go ahead with removing the Cancel button then. I'll dig into the code and look for the click-underlay stuff
OK @jmrodriguesgoncalves - I think I found what we need.
Check out this line here.
That's where hidePublishDialog() is defined, which is then passed into the new Underlay constructor on line 448 - when the Underlay is clicked, it removes the the underlay and fires that function - I tested this briefly with a console.log to make sure it works. I think you can add your logic there. Hope that helps - let me know.
@flukeout great stuff, never mind the latest 2 pushes, I will try this!
Yeah do what you need, you can just keep updating this PR, we can squash it all into one commit when we merge it.
@flukeout So currently, the code does everything that we decided to go with. It saves the description when the user clicks outside of the dialog box, and since the cancel button isn't there anymore, the user won't be as confused.
On the back-end however, the solution I used to make this work may not be the most elegant. In order to save the description, the function checks whether or not the project the user is working on is currently published or not. If it isn't, an ajax request is made to "unpublish" the unpublished project. This of course throws an error message, but the description is saved perfectly. If the project is published however, the code attempts an ajax request to publish with that same ID it is published on, changing only the description, which means nothing but that field will change. What this does is basically allow the user to change the description whether or not their project is published.
Hey awesome @jmrodriguesgoncalves - I'll test out the functionality now, and let's ping @gideonthomas so he can provide his thoughts on the back-end piece you asked about.
Edit: OK—I confirmed that the expected behaviour is happening! Nice.
@jmrodriguesgoncalves looks like there are some things you can fix in publisher.js, see https://travis-ci.org/mozilla/thimble.mozilla.org/builds/289235540#L483
@humphd got them sorted out, there were some spacing issues!
@flukeout, I haven't changed the logic from what you have tested so there shouldn't be any surprises, but let me know! This latest commit has the same functionality as before, just the spacing is now done correctly as to pass the two tests from below
OK, I'll wait on @humphd to weigh in on the changes you made and then I'll do one last functional test before we get this landed. Cheers!