Gradient toolbar enhancements / disable gradient editor

[John Smith]

Do we plan to disable the Gradient Editor ?
If so, this bug can track what modifications to the Gradient tool, Fill and Stroke dialog are needed.

ScislaC wrote (bug #592323)
> if the dialog doesn't get a rewrite during the 0.49 dev cycle, it will be disabled prior to release

Modifications needed to disable Gradient Editor dialog:
* newly-added gradient node not active (bug #627728) - Committed.
* on-canvas gradient editor focus problems (bug #179830) - Committed.
* Add the repeat combobox to the gradient tool toolbar (bug #171352) - Committed.
* Add the offset adjustment spinbutton to the gradient tool toolbar - Committed.
* Add the stop list and buttons to the gradient tool toolbar - Committed.

Other related issues:
* UI - Better Gradient Window (bug #722017)

[su_v (suv-lp)]

Other modifications needed (short-term):
* the context menu for custom swatches (solid color or gradient) in the 'Auto' palette opens the gradient editor dialog. Custom gradient swatches (multiple stops) currently not in use / visible cannot be edited with the gradient tool on-canvas (but are present in the 'Auto' palette).

See also:
<http://wiki.inkscape.org/wiki/index.php/Release_notes/0.48#Custom_Swatches>
<http://tavmjong.free.fr/INKSCAPE/MANUAL/html/Attributes-Fill-Stroke.html#Attributes-CustomSwatches>

AFAIU the implementation of custom swatches will probably change to make use of named/solid color properties accepted for SVG 2.0:
<http://www.w3.org/Graphics/SVG/WG/wiki/SVG2_Requirements_Mailing_List_Feedback#the_.3CsolidColor.3E_element>

[su_v (suv-lp)]

> * Add the offset slider to the gradient tool toolbar ?

Please also migrate the option to numerically position the offset stops.

[ScislaC (scislac)]

From a recent discussion (a few months back) on the devel-list, slightly updated text.

This relates to the attached mockup:
The 3rd gradient button is for Mesh gradients (apparently there will be a 4th as well for conical gradients), which exist in a branch currently. The following changes get other existing options in the user's view. Repeat is the function from the Fill & Stroke dialog to allow None, Repeat, or Mirror options. Following that is a Reverse Gradient button (Shift+R in the gradient tool, but should have UI), obviously not a real inkscape icon. Following are stop controls from the broken gradient editor (as ~suv pointed out it's mostly broken due to issues with canvas interaction). Buttons to Add & Delete stops (Insert & Delete keys respectively), the Stop Selector (which can be MUCH shorter, a single stop needs only a square of color, not a long rectangle), and the numeric Stop Offset widget (the slider makes no sense since you can slide the nodes directly on canvas). Note, all of the features already exist, they just need to be updated in the code and hooked up in the toolbar. Note the absence of the "Edit..." button (the Gradient Editor can be removed if all this functionality exists in the tool controls bar).

[John Smith (john-smithi)]

@ScislaC, thanks for the nice mockup.

Attached is a patch to test drive the features.
There are some small changes, around the wording and placement of buttons,
happy to move things around if preferred.

[ScislaC (scislac)]

I like the progress so far. I will chime in in the next couple days on some fine points as I already have observations and the tests were far too looses (sorry, weekends are a little tricky atm). However... I do need to say, you sir, are a gentleman and a scholar! :)

[su_v (suv-lp)]

<slightly off-topic>
I was wondering about the two button groups (toggles) for new gradients (part of the existing controls bar):
- Gradient Type: linear/gradient
- applied on: fill/stroke

When clicking on an inactive button, both button states in the group are toggled (to ensure that only one button is active). However, it is still possible to deactivate the current active button by clicking once on it, resulting in an inconsistent state with undefined behavior (to the user):
- With no gradient type selected, which type will be used?
- With neither fill nor stroke selected, which property of the object will have the gradient applied?
AFAICT the last used state (from the preferences (?)) is used, but the user has no obvious way to know in advance.

Maybe it would make sense to either
- add a check, and toggle both button states if one of them is deactivated, or
- use real radio switches (GtkRadioToolButton/GtkRadioButton (?)) to make sure that one button is always active.
</slightly off-topic>

[su_v (suv-lp)]

Offset adjustment/spinbutton:
With the current patch, the selected gradient stop is deselected after numerically changing the stop offset value (or pressing one of the spin buttons). Would it be possible to keep the same stop selected?

From the patch file:
- GtkWidget *button = sp_button_new_from_data( Inkscape::ICON_SIZE_DECORATION,
+ GtkWidget *button = sp_button_new_from_data( secondarySize,

When I had inquired about icons sizes for toolbar buttons in the past, I was told that buttons on the tool controls bar representing a 'state' (toggle buttons) originally had been intended to use a different (smaller) icons size then 'normal' buttons triggering actions (the gtk-icon-size 'inkscape-decoration' defaults to 12px if not explicitly changed in the system/user gtkrc file). Maybe someone on the mailing list could clarify (or remembers previous discussions, possibly with bulia byak).

+ gtk_container_set_border_width (GTK_CONTAINER (tbl), 8);

In this line, I locally changed the border width to '0' - using such a large border width for the gradient tool controls bar forces _all_ tool controls bars to use the same minimal height (in my tests, this enforced a larger height no matter which gtk theme I tested). It might be less noticeable with Ubuntu light themes like Ambiance/Radiance, because AFAICT the slider widget used e.g. for the tweak and calligraphy tool already extends the height for all_ tool controls bars due to the vertical size of the slider handle).

[John Smith (john-smithi)]

> new gradient toggle buttons ... it is possible to deactivate the current active button
Yes this doesn't make much sense. Should be converted to radio style buttons.

 > Would it be possible to keep the same stop selected?
Should be possible, but there are other similar feature requests (bug #627728) so maybe not easy. Let me check into it.

> 'state' (toggle buttons) originally had been intended to use a different (smaller) icons size
Currently it seems all the other toolbars use the same icon size for normal and stateful buttons. Hence i changed the gradient toolbar to be consistent with the other toolbars.

> forces _all_ tool controls bars to use the same minimal height
The goal here is consistency with buttons, comboboxes, spinbuttons and labels having the same height across toolbars. The gradient toolbar is inconsistent (on Ubuntu as least) since the New buttons are unusually small and the Change combobox/button unusually big. The aim was simply to make the Gradient size consistent with the other toolbars, this change should not affect any other toolbar.

[ScislaC (scislac)]

>> Would it be possible to keep the same stop selected?
> Should be possible, but there are other similar feature requests (bug #627728) so
> maybe not easy. Let me check into it.

Maybe not easy, however, the same was claimed for what you did with the gradient controls bar (by numerous people).

~suv: Regarding the size of state toggles, that is a design decision I don't understand. I think we're safe to ditch that approach at this point for a nicer and more consistent UI.

>> forces _all_ tool controls bars to use the same minimal height
> The goal here is consistency with buttons, comboboxes, spinbuttons and labels
> having the same height across toolbars. The gradient toolbar is inconsistent
> (on Ubuntu as least) since the New buttons are unusually small and the Change
> combobox/button unusually big. The aim was simply to make the Gradient size
> consistent with the other toolbars, this change should not affect any other toolbar.

The tool control bar (toolbox) design is strange. The toolbars do indeed affect each other visually. Everything is unfortunately held hostage by the Calligraphy, Eraser, and Tweak toolbars. Regarding the height issue, I brought up the new sliders in Gimp in a recent comment in a bug report for this very reason (well, size AND it has greater usability).
https://bugs.launchpad.net/inkscape/+bug/950508/comments/7

Now, for one other observation that ~suv didn't bring up is that it appears that the new toolbar is not made up of Actions like the other ones are. The best test for this using Inkscape in an unmaximized state (in fact, the smallest you can shrink the main window down) and viewing something like the Calligraphy tool controls bar and the arrow at the end which brings up a list of widget functions. The new calligraphy toolbar doesn't do that at this point.

Great work so far John!

[su_v (suv-lp)]

> Currently it seems all the other toolbars use the
> same icon size for normal and stateful buttons

AFAICT the four 'Affect:' buttons on the select toolbar use 'ICON_SIZE_DECORATION' in ink_toggle_action_new(), various mode buttons of the tweak tool, the icon for pressure sensitivity (on several controls bars), the icon for orthogonal mode on the connector toolbar, etc.

> since the New buttons are unusually small and (…)

This seems - at least partially - to be due to the gtk_box_pack_start() options. They differ for the two groups:
    gtk_box_pack_start(GTK_BOX(cvbox), cbox, TRUE, FALSE, 0);
<http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/view/head:/src/widgets/gradient-toolbar.cpp#L579>
    gtk_box_pack_start(GTK_BOX(cvbox), cbox, TRUE, TRUE, 3);
<http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/view/head:/src/widgets/gradient-toolbar.cpp#L621>
When testing last night (based on trial&error ;-) ), I got the same height as with other buttons/controls bars when using this box packing instead (for both groups):
        gtk_box_pack_start(GTK_BOX(cvbox), cbox, TRUE, TRUE, 0);

> (…) and the Change combobox/button unusually big.

Confirmed - apparently also due to different packing rules. Changing the border width does not really work: the ComboBox still extends to the max available height of the container (unlike other ComboBox widgets, e.g. the presets for the calligraphy tool, or the units selector, which appear to extend to their own minimal height, and do not stretch to the max available height of the container).

> The aim was simply to make the Gradient size consistent with the
> other toolbars, this change should not affect any other toolbar.

It does affect all toolbars (they reuse the same minimal height) - see also bug #392169. I had tested with Ambiance/Radiance, plain Gtk stock theme (Raleigh), and other Clearlooks- as well as Murrine-based themes, without and with custom settings added via ~/.gtkrc-2.0) - see attached screenshot with basic (stock) Gtk theme (without system or user gtkrc file) - current trunk on the left, with patched gradient tool controls bar on the right.

[John Smith (john-smithi)]

Have converted the whole gradient toolbar to use the standard action widgets. It should now look similar to the other toolbars for all themes without any sizing hacks. The toolbar overflow menu (when small main window) should also show the controls properly now. Converted the "new" buttons to proper radio buttons.

Still looking into the stop reselection and putting the gradient preview on the left of the combobox text.

Let me know if you see any more issues or ideas for improvement...

[ScislaC (scislac)]

Looking great! The only thing I will throw out there is the changed placement of the "reverse gradient" button is less intuitive imho. The original position made it more obvious that it worked on the gradient as a whole. The new placement makes me think I can select a couple gradient stops and it will reverse that segment/selection.

However, I do get the rationale since it also actually modifies the gradient definition... what is the best way to resolve this? (which may just be to leave as-is)

[ScislaC (scislac)]

Okay, so the behavior of the buttons Add & Delete stops is not what I would expect. Basically, if you select multiple stops on canvas and hit the Delete key on your keyboard, it deletes all selected stops. The button deletes the last selected stop only. Similarly, for add stops, if I have three stops in a row selected on canvas and hit the Insert key, it add stops in between the selected stops (meaning two stops get added). With the same selection, the Add stops button only adds a stop between the last selected stop and the one following it. A big bonus to this working "as expected" is that it gives new functionality to our friends with Macs as they apparently don't have an Insert key.

A related item, if you have a gradient on the fill and a gradient on the stroke and neither gradient selected it detects this and marks says "Multiple selected" in the gradient chooser. Perhaps when there are multiple stops selected the stop chooser should do the same (and not track or dismiss last selected for adding/deleting).

There are other behaviors which are bad which are exhibited with Multiple selected gradients, but those quirky behaviors already existed and are outside the scope of what you set out to do here... so I'll see if I can find an existing report to comment on for that stuff. :)

[John Smith (john-smithi)]

Thanks for testing ScislaC, updated patch attached.

Moved the reverse button to the original position, see how it looks ...
Add/Delete buttons should now match the keyboard commands.
Added "Multiple Stops" to the combo, add/delete should remain available.

[ScislaC (scislac)]

Looks great imho. Plus, bonus fix of it maintaining selection after clicking to Add stops (or using shortcut). :) Awesome work John... afaict this is definitely ready for wider testing in trunk, may want to wait for ~suv to chime in though.

[su_v (suv-lp)]

1) Reverse gradient - Missing feature:
The keyboard shortcut works on selections of multiple gradients (at least one stop of a fill or stroke gradient is selected), optionally across several selected objects. Could this feature be added to the toolbar button as well?

2) Reverse gradient - New feature request (partially based on item 1):
Would it be feasible / considered useful to keep the selection of gradients/stops after pressing 'Reverse'?
Use case 1: an object with gradients on both fill and stroke (not aligned)
- Without selecting one of the two gradients, both will be reversed at the same time.
- By selecting a stop of one of the gradients, the direction of the selected gradient only is reversed.
It could be useful to keep this selection active after 'Reverse'.
Use case 2: a selection of multiple stops of (different) gradients on multiple objects
The user might want to continue to edit the selected stops of different gradients after reversing the selected gradients, or reverse them again to restore the prior state.

3) Radial gradient - stop chooser:
If the center of a radial gradient (start stop) is selected, the chooser shows 'Multiple stops' - this is ambiguous (there is only one stop at the center of the radial gradient).

4) Radial gradient - Insert new stop:
If the center of a radial gradient (start stop) is selected, and a new stop is inserted (by pressing the '+' button on the toolbar), two new coinciding stops are inserted (expected result would be a single new stop). I cannot test whether the existing keyboard shortcut exposes the same behaviour (counting the center stop twice (?)).

5) Order of buttons:
What about moving the 'Reverse' button further to the left, between the 'Select:' and 'Repeat:' comboboxes? The gradient preview of the 'Select:' combobox immediately reflects the change of the gradient direction, whereas the 'Repeat:' option is not affected.

[su_v (suv-lp)]

> 1) Reverse gradient - Missing feature:

Oops - didn't test carefully enough, sorry. Currently the keyboard shortcut works by reverting all gradients of the selected objects (select multiple objects with the select tool, switch to gradient tool, and apply 'Shift+R'), but not on a selection of individual gradients within the current selection of object (it only reverts the first selected gradient).

Item 1 would be a feature request as well (if feasible/considered useful).

[ScislaC (scislac)]

#4 is confirmed with the Insert key in addition to the Add stop button. It does indeed add two stops at the same location (which is not expected nor desired). However, it is reproduced with the 0.48.x branch as well and is apparently an existing bug. Desirable to be fixed but not necessary.

[su_v (suv-lp)]

6) Insert new stop
With 950677.v3.patch, the '+' button is active (sensitive) if no gradient stop is selected, but does not insert a new stop when pressed (it requires at least one stop to be selected).

Proposal: Either make the button insensitive if no stop is selected, or - if none is selected - default to inserting a new stop after the start stop.

[John Smith (john-smithi)]

Great testing ~suv, thanks !

New patch attached:

> 1) The keyboard Reverse gradient shortcut works on selections of multiple gradients
Reverse button is now enabled with multiple selections, should work in the same way as the keyboard shortcut.
Ill look into selection of individual gradients reverse feature request as well.

2) Keep the selection of gradients/stops after pressing 'Reverse'
Still looking into this ...

3) Center of a radial gradient is selected, the chooser shows 'Multiple stops'
This should be fixed

4) Pressing the '+' button with center of a radial gradient selected, creates 2 new coinciding stops
This should be fixed. FYI There are actually 2 stops in the center of a radial (center and focus) hence the "multiple" issues.

5) What about moving the 'Reverse' button to the left of the Repeat
Done, looks ok to me.

6) The '+' button is active if no gradient stop is selected
This should be fixed, '+' only active when a stop is selected (same as delete button)

If there are no major issues, i will commit this for wider testing.

[ScislaC (scislac)]

No major issues. If you have a desire to fix bugs that carried over from the previous implementation we can point those out, but I'm sure there are other things you're interested in working on. :)

[John Smith (john-smithi)]

Committed as r11134.

[John Smith (john-smithi)]

I have committed some other gradient related fixes as well (bug #179830, bug #627728).
Any other major gradient issues ?

Should we now disable the Gradient Editor - when double clicking a stop (would still be available via the Fill and Stroke dialog) ?

[John Smith (john-smithi)]

Committed r11216 - Disabled Gradient Editor when double clicking on a stop (knot).

[John Smith (john-smithi)]

Patch for linking gradients (this is the behavior of the Gradient Editor) - (see bug #722017 comment 20+)

There are 2 toggle buttons on the toolbar to toggle link/unlink of gradients so you can modify all related gradients at once.
It works for changing stop colors, moving middle stops and reversing gradients.

It does not work for Gradient repeat patterns - it seems you cant have a repeat pattern on a "master" gradient.
What is the preferred behavour here ?
1. Disable the repeat combo when "linked" is selected, or
2. Allow changing the repeat pattern only for the selected gradient , even when linked is selected ?

Reused some node toolbar icons for linked/unlink buttons - are these ok, or should we change icons ?

[ScislaC (scislac)]

In testing, I ran across some unexpected behavior. I have attached a file showing the issue (not that the gradient tool should be set to linked).

If you select the bottom object, change to gradient tool, and add a new gradient stop on canvas, it does not show up. If you change your selection to the top object, it displays the new stop, change selection back to the bottom object and it is now visible. If you try to add stops with the same method to the top object, they show immediately.

About all I can tell you is that the top object was created first, the bottom object created second. First object has the gradient, assigned the gradient to the second object after the fact.

As for button... I think it should be a singular button with two states (similar to the lock on the selector tool), with no label, and placed directly to the right of the gradient chooser. It would also help reduce confusion about the repeat options imho. The icons don't really fit, so I will create proper link/unlink icons once it lands in trunk (I'd suggest using the lock in the meantime though).

[John Smith (john-smithi)]

> New gradient stops not showing up
Good find, should be fixed in this patch ... btw seems to be long existing timing issue, can reproduce on the trunk as well.

> Singular button with two states
Yes i agree, have changed it to a single toggle button with the "lock" icon.

If there is no concern about the "repeat" option when locked, any other issues before committing ?

[ScislaC (scislac)]

Awesome, works as expected now. I don't think the repeat option is an issue as it's really a per object assignment, like gradient placement/angle/direction. I personally think it's good to commit to trunk. Great work as always!

[John Smith (john-smithi)]

Committed revision 11291 - Add link option to gradient toolbar.

[su_v (suv-lp)]

Minor usability issue with the gradient selector on the toolbar (related to bug #973195):

The auto-generated IDs of (forked) gradients increase in string-length each time a fork occurs. Depending on the workflow this can result in really long ID strings, which in turn affects the width of certain UI elements.

Please open attached example (166 forked gradients, generated by a script which repeatedly copies & pastes the last pasted object), and test selecting a different gradient from the GtkComboBox menu…

Proposal:
Would it be feasible to display the gradient GtkComboBox as list (with scrollbars) instead of a menu, thus keeping a fixed width no matter how long the IDs of the currently used gradients are? The font selector in the controls bar of the text tool works like this AFAICT.

[John Smith (john-smithi)]

> display the gradient GtkComboBox as list (with scrollbars) instead of a menu
That is not so easy to do, especially for the toolbar since the GtkComboBox is not used directly. Instead a custom control that dynamically changes depending on the toolbar state - such as when the toolbar doesn't fit on screen and converts to a menu - which has the same overflow issues.

The simpler option is to "ellipse" the text - ie truncate part of the name so it always fits in a maximum width - maybe 40 chars.
See attached truncating the middle of the name with ...

[su_v (suv-lp)]

> That is not so easy to do, especially for the toolbar
> since the GtkComboBox is not used directly.

What is different from the text tool controls bar? I don't really understand why it is doable in the controls bar of one tool (text tool), but not in the controls bar of another tool (gradient tool).

Note: my comment here was only about the gradient tool and its controls bar, and not about the gradient selector in the fill&stroke dialog.

[John Smith (john-smithi)]

Yes you may well be right, the Font selector control could be used, its really a matter of how much work ...

* The scroll-bars on the Font selector dont work under Ubuntu/Unity (no surprise maybe)
* Not sure yet if the Font control works like the other controls converting to a overflow menu if it can't fit on screen
* If the overflow menu does work, how do you deal with the menu items then being very long ?

Of course anything can be done, its just a trade-off between the amount of work vs the benefit.

Ellips'ing the text is very little effort, and could be committed right now.

[ScislaC (scislac)]

Imho, the font selector is a nasty and unfriendly hack. I'd vote for ellipsing in the meantime to get something better in place.

[su_v (suv-lp)]

> Ellips'ing the text is very little effort, and could be committed right now.

That's fine with me :-)

[John Smith (john-smithi)]

Committed revision 11365 to trunk.

Gradient toolbar and Fill/Stroke gradient names should now be ellipsed if over 35 chars (this seemed to be about the max size without increasing the width of the Fill/Stroke dialog on Ubuntu).

Should also partially fix the issues in bug #973195.

[John Smith (john-smithi)]

Revision 11370, fix for ellipsize function.

[John Smith (john-smithi)]

r11519 : Gradient editor default to off.
F&S Gradient edit button now invokes the Gradient tool.

The legacy Gradient editor dialog can still be used by setting the preference "/dialogs/gradienteditor/showlegacy" to "true"

[John Smith (john-smithi)]

r11523 : Preference Tools->Gradient->"Use legacy Gradient Editor"