Closed Bug 886211 Opened 9 years ago Closed 8 years ago

Provider capability for number of categories supported

Categories

(Calendar :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Fallen, Assigned: Fallen)

References

Details

Attachments

(1 file, 2 obsolete files)

Bug 316927 added multiple category support to the UI, but there is no way for a provider to specify if only one category is supported. This needs to be added.

See the capability for maximum number of alarms for an example.
Hey Daniel, any updates? Could you take care of this bug?
Hello Philipp,

I'm sorry, I'm quite busy now and won't be able to take of this bug anytime soon.
Ok, thanks for the update. Reassigning to nobody, but we really need someone to take care of this in the next two weeks.
Assignee: dagomez → nobody
Status: ASSIGNED → NEW
Attached patch categories-capability.diff (obsolete) β€” β€” Splinter Review
This patch should take care and covers the following cases:

* zero categories supported: Hides the category box. 
  -- Calendar dropdown gets insanely large, but I wouldn't know what 
     to do about it right now. collapsing the box makes it look 
     strange, disabling seems unnatural to me too?
* one category supported: Shows no checkboxes and single-select
* Specific number of categories: when number is reached, other checkboxes
  are disabled
* Any number of categories (null, or -1)

You can test this by using the ics inspector extension (version 0.7 works, it should be on the AMO other versions page until its reviewed) and right clicking on a storage calendar, then evaluate javascript and enter: 

> target.setProperty("capabilities.categories.maxCount", value);

Where value is the max count to set.

I'd like to see this in Lightning 2.6, so I'd appreciate if you could give it a test run. Merge date is August 5th.
Assignee: nobody → philipp
Status: NEW → ASSIGNED
Attachment #783436 - Flags: review?(matthew.mecca)
Comment on attachment 783436 [details] [diff] [review]
categories-capability.diff

Review of attachment 783436 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good overall. r=mmecca

One minor issue: with max categories > 1, if the maximum amount of categories set, then the event is saved and re-opened, one additional category can then be selected.

::: calendar/base/content/widgets/calendar-widgets.xml
@@ +50,5 @@
>          <getter><![CDATA[
>            let categoryListbox = document.getAnonymousElementByAttribute(this, "anonid", "categories-listbox");
> +          if (this.maxCount == 1) {
> +            let selectedItem = categoryListbox.selectedItem;
> +            return selectedItem ? [selectedItem.getAttribute("value")] : []

missing a ';'

Also, the indentation of the javascript in this file doesn't match the rest of the file.
Attachment #783436 - Flags: review?(matthew.mecca) → review+
Attached patch Fix - v2 (obsolete) β€” β€” Splinter Review
Thanks, good catch! I've also noticed UX is a bit strange when switching from a calendar with 5 allowed categories to one with 3 allowed categories: the categories are initially still selected, when you unselect them they are disabled.

This is an edge case and would probably require some l10n string as explanation, so I think its ok to leave it as is. I think most providers will probably allow 1 or any number of categories.
Attachment #783436 - Attachment is obsolete: true
Attachment #785162 - Flags: review+
Attached patch Fix - v3 β€” β€” Splinter Review
Ah we have some documentation in calICalendar.idl. I've updated the documentation, its formatting and also added the alarms maxCount capability which was missing.
Attachment #785162 - Attachment is obsolete: true
Attachment #785170 - Flags: review+
Comment on attachment 785170 [details] [diff] [review]
Fix - v3

And we also need this on aurora so its part of Lightning 2.6
Attachment #785170 - Flags: approval-calendar-aurora+
Comment on attachment 785170 [details] [diff] [review]
Fix - v3

Also approving for beta since the merge has happened.
Attachment #785170 - Flags: approval-calendar-beta+
https://hg.mozilla.org/comm-central/rev/88226d7b1bb9
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.6
You need to log in before you can comment on or make changes to this bug.