Community
Participate
Working Groups
TaskAttributes should have a notion of "required". Unlike Bugzilla, some connectors have required fields for new task creation that can't be prefilled with some value. Currently, all connectors handle this internally by somehow annotating the UI to help the user and checking for required fields on submission. I could imagine that a metadata attribute similar to read-only would be sufficient for 80% of the cases.
Pushed initial draft to http://review.mylyn.org/509
Created attachment 214778 [details] mylyn/context/zip
Thanks Benjamin. The review is now at https://git.eclipse.org/r/#change,6024. I plan on taking a look once the Juno release is settled.
Reviews continued on https://git.eclipse.org/r/#/c/19677/ and https://git.eclipse.org/r/#/c/19678/
I wonder if we should decorate required fields all the time or only indicate an error when they don't have a value?
For now, only decorate when the a required field is empty. This we avoid having 2 types of asterisks in the editor meaning different things, and we avoid having 2 icons denoting requiredness.
Thanks very much, Benny and Leo, for the contribution!
I noticed a few small nits when I tried out the latest task editor: * When I opened the editor all editors for required attributes were flagged with error markers. I thought these would only show on focus? * Boolean attributes were showing the error marker even though they had a valid value (i.e. false in that particular case). Leo, what are your thoughts?
Created attachment 238962 [details] cut off error marker on Mac
(In reply to comment #8) > I noticed a few small nits when I tried out the latest task editor: > > * When I opened the editor all editors for required attributes were flagged with > error markers. I thought these would only show on focus? If we did that, users would have to select each attribute to find out whether it was required.
(In reply to comment #9) > Created attachment 238962 [details] > cut off error marker on Mac Could be that the padding between label & field is smaller when you run on Mac without the "-Dorg.eclipse.swt.internal.carbon.smallFonts" flag.
(In reply to comment #8) > * Boolean attributes were showing the error marker even though they had a valid > value (i.e. false in that particular case). > > Leo, what are your thoughts? I suspect that if a boolean attribute has no value, the BooleanAttributeEditor will still display a default of false, without sending any modification event. I'd need to look into it further.
I wonder if this affects single select attributes as well.
(In reply to comment #13) > I wonder if this affects single select attributes as well. I suspect this would be the case for any single select attribute where the attribute mapper generates a non-empty default.
Can we check before displaying the icon that the attribute editor has no value?
(In reply to comment #15) > Can we check before displaying the icon that the attribute editor has no value? We were, but the check is incorrect for boolean attributes! Here's a review: 21654: 378032: fix for boolean attribute editors that show required decorator [Ie343ba26] https://git.eclipse.org/r/#/c/21654/
(In reply to comment #11) > (In reply to comment #9) > > Created attachment 238962 [details] > > cut off error marker on Mac > > Could be that the padding between label & field is smaller when you run on Mac > without the "-Dorg.eclipse.swt.internal.carbon.smallFonts" flag. I am actually seeing some clipping on Windows too, although not as bad as that. For me there is only 1 pixel cut off whereas it looks like for Steffen there are 3. Here's a fix that shifts the editor control 3 pixels to the right: https://git.eclipse.org/r/21861 I suppose we could make the amount of shifting dependant on the OS but I'm not sure it's worth it for 2 pixels. I also noticed that AttributeEditorToolkit.createValidator puts the error icon at SWT.BOTTOM whereas here we use TOP. They should probably be the same. My preference is to change AttributeEditorToolkit to use TOP since I think that is the convention and looks better.
It should be noted that without the above fix, the decorators created by AttributeEditorToolkit are also clipped. In hindsight we should perhaps have used AttributeEditorToolkit to do these decorations as well but we won't have time to change that before the release.
(In reply to comment #17) > I also noticed that AttributeEditorToolkit.createValidator puts the error icon > at SWT.BOTTOM whereas here we use TOP. They should probably be the same. My > preference is to change AttributeEditorToolkit to use TOP since I think that is > the convention and looks better. Isn't top already used by the content assist hint?
Ah, good point! I guess we should use BOTTOM instead so that they don't conflict.
I've updated https://git.eclipse.org/r/21861 to address clipping in the people part as well as the attributes section, and to change the error icon to use BOTTOM. Please verify the fix.
Created attachment 240046 [details] decorators with big fonts
Created attachment 240047 [details] decorators with smallfonts
I've posted a couple of screen grabs of the required & content assist decorators together on the Mac with smallfonts enabled and disabled. The smallfonts case, the decorators overlap slightly on to each other, but that may just be a case of the Mac text field not being tall enough to accomodate two 8*8 pixel icons. The decorators no longer get clipped by the text field though.
Thanks for verifying. I merged the fix.
(In reply to comment #8) > * Boolean attributes were showing the error marker even though they had a valid > value (i.e. false in that particular case). Actually, the attribute did not have any value but the attribute editor has no way to show that so it appeared false. But the task can't be submitted without checking and unchecking the boolean, so since we suppressed the error decorator, we should also initialize the attribute: 43689: BooleanAttributeEditor should initialize attribute to false [I29c92306] https://git.eclipse.org/r/#/c/43689/
