Community
Participate
Working Groups
It would be nice to have a provider (similar to the label or font or color providers) to determine the checked/grayed state of elements in a CheckBox*Viewer (either Table or Tree). This would generally simplify the programming model and give the developer better control over how the model objects are displayed in the viewer.
Created attachment 63105 [details] The ICheckedStateProvider interface The ICheckedStateProvider interface defines the necessary functionality of a provider that will determine the checked/grayed state of a model object as well as respond to the (un)checking/(un)graying of a model element.
Created attachment 63106 [details] A subclass of the CheckboxTableViewer The attached subclass of the org.eclipse.jface.viewers.CheckboxTableViewer demonstrates how the viewer could/would make use of the ICheckedStateProvider. This class is currently being used in our product offering.
*** Bug 183539 has been marked as a duplicate of this bug. ***
*** Bug 9390 has been marked as a duplicate of this bug. ***
*** Bug 45795 has been marked as a duplicate of this bug. ***
(In reply to comment #1) > Created an attachment (id=63105) [details] > The ICheckedStateProvider interface I just looked at this attachment and saw the copyright header. This is in conflict with the terms of use (http://www.eclipse.org/legal/termsofuse.php): "any Contributions, as defined in the applicable license(s), uploaded, submitted, or otherwise made available to the Eclipse Foundation and/or the Members, by you that relate to such Content are provided under the terms and conditions of both the CPL and the EPL and can be made available to others under the terms of the CPL and/or the EPL." Could you please clarify the license situation, and upload updated versions of the files if you want to contribute them under EPL, or mark them as obsolete if you don't want to contribute? Thanks.
Created attachment 79753 [details] Modified ICheckedStateProvider.java Sorry about the previous attachment. It was really meant just as an example of how I did it. Here is another copy with all of the copyright/license stripped out. The other attachment ("A subclass of the CheckBoxTableViewer") really is not contributed to Eclipse per se, so I don't think I need to remove the copyright/license terms. It really is just a sample of an implementation. It is meant to give you a starting point and just an idea of one way of providing the desired functionality. I hope this helps.
Adding a reference to Bug 205293 as this is easy to hit as the javadoc is not clear for setCheckedElements().
*** Bug 157324 has been marked as a duplicate of this bug. ***
Mass update - removing 3.4 target. This was one of the bugs I marked for investigation (and potential fixing) in 3.4 but I ran out of time. Please ping on the bug if fixing it would be really important for 3.4, and does not require API changes or feature work.
*** Bug 180598 has been marked as a duplicate of this bug. ***
Our biggest issue with the current setup was performance. On a tree with ~1000 elements (2 levels, ~20 on the root level) calling setCheckedElements took ~25 seconds when most of the elements are checked. I tried moving the setChecked logic to the ColumnLabelProvider.update(ViewerCell) method, but the current CheckboxTreeViewer implementation clears the check state (in preservingSelection->applyState). The solution I used was to copy CheckboxTreeViewer and strip out all of the logic except for the ICheckStateListener support (see attached CustomCheckboxTreeViewer). Then I extend ColumnLabelProvider to set the check state in update(ViewerCell): @Override public void update(ViewerCell cell) { super.update(cell); Widget widget = cell.getItem(); if (widget instanceof TreeItem) { TreeItem item = (TreeItem)widget; Tree parent = item.getParent(); // Make sure it's a checkbox tree if ((parent.getStyle() & SWT.CHECK) != 0){ Object element = cell.getElement(); boolean checked = isChecked(element); item.setChecked(checked); item.setGrayed(isGrayed(element)); } } } /** * @param element * @return whether the element should appear grayed */ protected abstract boolean isGrayed(Object element); /** * @param element * @return whether the element should appear checked */ protected abstract boolean isChecked(Object element); That's it, simple and clean. And substantially faster than the previous version. It went from ~25 seconds to no noticeable delay. This is due to the fact that TreeItems are only created when requested by the user instead of having to create them for every element supplied to setCheckedElements. I'm not sure how this will fit into eclipse due to the api change to CheckboxTreeViewer, but I offer it as a solution to interested parties.
Created attachment 105514 [details] Alternate implementation for CheckboxTreeViewer
*** Bug 247787 has been marked as a duplicate of this bug. ***
The following references is copy from the duplicated bug 247787: >>Another issue I would like to say is, why we need 2 classes(CheckboxTableViewer >>and TableViewer) instead of just one TableViewer class? I just take a look the >>source of CheckboxTableViewer. It's simple. Why don't we move the code from >>CheckBoxTableViewer into TableViewer(and leave an empty CheckboxTableViewer >>class, for compatibility), so we could just specified a SWT.CHECK style bit to >>the TableViewer's constructor and then we get checkbox feature supported by the >>TableViewer, further, we can inspect the checkbox suppport by test >>((TableViewer.getStyle() & SWT.CHECK) != 0)? >>------- Comment #3 From Boris Bokowski 2008-09-22 10:51:48 -0400 ------- >(In reply to comment #2) >> Another issue I would like to say is, why we need 2 classes(CheckboxTableViewer >> and TableViewer) instead of just one TableViewer class? I just take a look the >> source of CheckboxTableViewer. >CheckboxTableViewer has additional behaviour, such as preserving the checked >state of items across a refresh operation. We can check style for SWT.CHECK in the appropriate method to determine whether we want to do a checked state preserving or not. for example, public class TableViewer ... { ... public boolean hasCheckboxSupport() { return (getTable().getStyle() & SWT.CHECK) != 0; } protected void preservingSelection(Runnable updateCode) { if (!hasCheckboxSupport()) { preservingSelection(updateCode, false); return; } TableItem[] children = getTable().getItems(); CustomHashtable checked = newHashtable(children.length * 2 + 1); CustomHashtable grayed = newHashtable(children.length * 2 + 1); for (int i = 0; i < children.length; i++) { TableItem item = children[i]; Object data = item.getData(); if (data != null) { if (item.getChecked()) { checked.put(data, data); } if (item.getGrayed()) { grayed.put(data, data); } } } preservingSelection(updateCode, false); //super.preservingSelection(updateCode); children = getTable().getItems(); for (int i = 0; i < children.length; i++) { TableItem item = children[i]; Object data = item.getData(); if (data != null) { item.setChecked(checked.containsKey(data)); item.setGrayed(grayed.containsKey(data)); } } } }
I think the underlying question here is whether checked state should follow the JFace convention of using a provider or not. Currently it is tracked more like selection which makes sense for certain use cases. For example, a dialog with a checkbox tree to have the user select from a set of options. This provides a better user interface than using selection which can be lost easily by forgetting to Shift- or CTRL-click. Where this pattern becomes a problem is when you want check state to reflect some aspect of the model (e.g. visibility). This is much more similar to the use case for a label provider: present to the user some information about an element. The current JFace implementation makes this very difficult. The developer must reset the checked state any time the viewer is refreshed. This is in contrast to the Provider pattern that is used throughout JFace. I would like to see JFace support both patterns if possible to provide the best flexibility for developers.
(In reply to comment #16) > Where this pattern becomes a problem is when you want check state to reflect > some aspect of the model (e.g. visibility). This is much more similar to the > use case for a label provider: present to the user some information about an > element. The current JFace implementation makes this very difficult. The > developer must reset the checked state any time the viewer is refreshed. This > is in contrast to the Provider pattern that is used throughout JFace. That is the very case I'm struggling with also. Unfortunately, the JFace data binding ViewersObservables.observeCheckedElements() works with this philosophy, too... It would be urgent to resolve this.
Created attachment 117775 [details] ICheckStateProvider - interface, modified Check*Viewers, tests Work on this problem has been done in bug 182714 and here is the final product! It defines the interface ICheckStateProvider which is currently supported by CheckTreeViewer and CheckTableViewer. Upon any refresh, the ICheckStateProvider is asked if a given element is checked or grayed, and the item in the viewer set accordingly. Included also are tests.
Matthew's patch is in HEAD now. Keith, Cole, I was hoping for feedback from you (should have written that here, I know). Would you be able to have a look at the new API and say whether it would work for your use cases?
Yes, that looks great for our purposes. When we finally move to 3.5 as our target platform we will start using it.
