STR: 1. Make sure you have two writable calendars A and B. 2. Open an event on calendar A. 3. Switch the calendar dropdown to calendar B. 4. Mark calendar A readonly 5. Switch the dropdown back to calendar A. Actual Result: * Most (but not all) dialogs controls are disabled Expected Result: * Either we should really disable all controls, but I'd prefer the next option: * After Step 4, a notification bar should show at the top telling the user he cannot save the event on the current calendar since its not writable. * While bar is showing, disable all save related controls and when closing.
The proposed solution sounds very elegant. Only one question arises: Should we really let someone edit an event that can not be saved at the moment? I think yes, but maybe we should add an advice shown that one can save it if he changes to a writable calendar. Marking uiwanted to get some more feedback.
Some additional comments: The choose-calendar control is also disabled, so one can't switch back to a writable calendar. when canceling the dialog on a read-only calendar, the confirmation asks "save, don't save or cancel?". This also has to be changed.
Created attachment 443066 [details] Screenshot of the proposed dialog This is a screenshot of how the Dialog will look.
Created attachment 445991 [details] Screenshot of the close-Prompt if item was changed Since the event can not be saved in a read only calendar, the confirmation prompt on canceling the dialog has to be different. I thought about just leaving out the "save" button, but with the prompt if you want to save, this is confusing.
Created attachment 445995 [details] [diff] [review] Patch V1 The patch has one known problem: If the current selected calendar in the event-dialog changes it's status to "read-only", the dialog does not get updated. A similar problem was already adressed in a comment to bug 402421  and I think this is related. This will be checked in after 1.0b2, because of the string changes. : https://bugzilla.mozilla.org/show_bug.cgi?id=402421#c8
Adding andreas who's going to take over the review
This is a good step forward, as you no longer end up in a dead-end-street where you can't even switch back to the writable calendar. I'm wondering though, are you sure you even want to be able to switch to a unreadable calendar from this dialog? As far as I can tell, this is the only way to get to a new event in an unwritable calendar, so what about this: Since you're doing a check if it's readable or not, would it be possible to do that on the dropdown click and just gray out the unwritable calendar there?
Sounds reasonable. The only case when this is'nt enough, would be if we get the notification to this dialog, when a calendar gets unwritable while editing an event. If this is some time implemented, we would have to think again. Should we have that in mind now or is it likely, that this notification will never be omplemented?
Comment on attachment 445995 [details] [diff] [review] Patch V1 Since the behavior should be slightly changed, r- on this patch. Keeping ui-r? to answer comment 8.
Comment on attachment 445995 [details] [diff] [review] Patch V1 Some comments on the code of the current patch: >+ // increment choice by one, because the prompt has one button less. >+ choice += 1; choice++; >+askSaveTitleReadOnly=Can not Save I believe the correct wording is "Cannot Save", maybe a native speaker can confirm. >+askSaveMessageReadOnly=Current calendar is Read only. Do you want to close the dialog and discard changes? Please check how we've written it elsewhere and whats correct: read only vs readonly vs read-only vs ... In any case, read needs to be lower case, and I'd start with "The current calendar...".
(In reply to comment #8) > Sounds reasonable. > The only case when this is'nt enough, would be if we get the notification to > this dialog, when a calendar gets unwritable while editing an event. > If this is some time implemented, we would have to think again. > > Should we have that in mind now or is it likely, that this notification will > never be omplemented? Perhaps we should do that as a new bug report in that case. Making it impossible to select in the first case as I described in comment 7 would take care of the issue described in this bug.
Comment on attachment 445995 [details] [diff] [review] Patch V1 Giving ui-r minus on this, as I think it can be done in a more elegant way.
How do we continue on this one? Is the proposal from comment 7 a solution that can be agreed on?