Review Request: change Sheets UI by moving the cell editor out of the cell tool

C. Boemann cbr at boemann.dk
Tue Apr 24 10:04:37 BST 2012


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104708/#review12859
-----------------------------------------------------------


First of all: I like this initiative very much. I have some feedback, but please understand that my general impression is VERY positive. But here goes with all the critique:

The Cell Editor docker: It should have it's content expand to fill the height of the docker. And the buttons to the left should be top aligned.

The formatting buttons: The un-merge button seems to be stuck in enabled mode, and we have fdo names for these icons and there exist oxygenversions.

The option widgets of the cell tool: You should add SpecialSpacer widgets to the bottom of the option widgets, so all the buttons remain top aligned.

Interaction design: The cells are not selectable when in anything but the cell tool. I find that annoying. It's not a new thing but now that you have the cell editor always visible and working it becomes more apparent. I wonder if you should not move the cell selection from the cell tool, to something the canvas does, if the mouse events are not picked up by the tool system. I'm not sure wether this can wait until after this has been moved to master, as I think the issue is a ui disaster (note: standard interaction design term for something that leaves the user in trouble, not as rude as it sounds) and kind of makes sheets unreleasable. If you do this, then the Cell tool becomes a "Cell Formatting" tool

Modebox instead of toolbox: I think this would be a good time to use the modebox Words uses rather than the toolbox that is more applicable to drawing applications. It's not a big change code wise: 10 lines or so. Doesn't have to be part of this branch but I really think it wold make sense to do it together.

The scroolbar focus: no objection

I'm ready to help with further (implementation) details.


- C. Boemann


On April 24, 2012, 5:32 a.m., Marijn Kruisselbrink wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104708/
> -----------------------------------------------------------
> 
> (Updated April 24, 2012, 5:32 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> This changes to UI of Calligra Sheets a bit by moving the cell editor widget out of the CellTool and instead have it as a separate docker. On top of that there are several new option widgets for the cell tool that provide common formatting options.
> I've also changed the KoToolDocker a tiny bit, by changing the scroll area in it to not be able to get keyboard focus, at least for these new dockers that is preferred, I'm not sure if there are any other tool option widgets/use cases where you would want the scroll area containing the tool option widgets to be able to get keyboard focus?
> Please test (also available in the tables-uirefactor-mek branch) and see if anything weird is happening since it is a fairly big change.
> 
> 
> Diffs
> -----
> 
>   libs/main/KoToolDocker.cpp 44ce789 
>   sheets/CMakeLists.txt 5355719 
>   sheets/Messages.sh e636ac1 
>   sheets/part/CanvasItem.h 41acefe 
>   sheets/part/CanvasItem.cpp c5ccba3 
>   sheets/part/Factory.cpp fb612da 
>   sheets/part/View.h 5322e43 
>   sheets/part/View.cpp 5c11f98 
>   sheets/ui/ActionOptionWidget.h PRE-CREATION 
>   sheets/ui/ActionOptionWidget.cpp PRE-CREATION 
>   sheets/ui/CellEditorDocker.h PRE-CREATION 
>   sheets/ui/CellEditorDocker.cpp PRE-CREATION 
>   sheets/ui/CellToolBase.h aa3a094 
>   sheets/ui/CellToolBase.cpp 33f2d29 
>   sheets/ui/CellToolBase_p.h 7750a4a 
>   sheets/ui/CellToolBase_p.cpp 0cba586 
>   sheets/ui/CellToolOptionWidget.h c46b7df 
>   sheets/ui/CellToolOptionWidget.cpp 4a1d068 
>   sheets/ui/CellToolOptionWidgets.xml PRE-CREATION 
>   sheets/ui/ExternalEditor.h ab3ee6d 
>   sheets/ui/ExternalEditor.cpp e9f8781 
>   sheets/ui/LocationComboBox.h e666ada 
>   sheets/ui/LocationComboBox.cpp 6215039 
>   sheets/ui/Selection.h baa9cdf 
>   sheets/ui/Selection.cpp 316a2fc 
> 
> Diff: http://git.reviewboard.kde.org/r/104708/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Marijn Kruisselbrink
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20120424/bbaa9018/attachment.htm>


More information about the calligra-devel mailing list