Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow ability to define style cycles for Draw tools #3612

Merged
merged 5 commits into from Apr 28, 2019

Conversation

Projects
None yet
3 participants
@philippjfr
Copy link
Contributor

commented Apr 9, 2019

This PR allows adding style cycles to the PolyDraw stream (but this could be expanded to the other draw streams). As an example here we cycle over the fill_color and line_width of the drawn polygons:

poly = hv.Polygons([[(2, 2), (5, 8), (8, 2)]])

poly_stream = streams.PolyDraw(source=poly, drag=True, show_vertices=True,
                               style_cycles={'fill_color': ['red', 'green', 'blue'],
                                             'line_width': [3, 5, 10]})

poly

style_cycles

  • Fixes #2694
  • Expand to BoxEdit and FreehandDraw tools
  • Add example in documentation
@jbednar

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2019

Looks great! Does it need to be style_cycles, or could it just be styles, which are cycled if they are cycles and otherwise just applied? Just brainstorming; not sure which approach is better.

@philippjfr

This comment has been minimized.

Copy link
Contributor Author

commented Apr 10, 2019

Looks great! Does it need to be style_cycles, or could it just be styles, which are cycled if they are cycles and otherwise just applied? Just brainstorming; not sure which approach is better.

It could just be styles I suppose, that is somewhat redundant since you can just set the style in the usual way but may be cleaner regardless.

@philippjfr

This comment has been minimized.

Copy link
Contributor Author

commented Apr 10, 2019

@jbednar @jlstevens Let me know which you prefer, I'd be happy with accepting both lists (as cycles) and scalar values, and then call it simply styles.

@philippjfr philippjfr added this to the v1.12.1 milestone Apr 10, 2019

@jbednar

This comment has been minimized.

Copy link
Contributor

commented Apr 10, 2019

I'd vote for styles. Actually I vote for just respecting the usual style options set in the usual way and cycling if those are cycles, but I assume that's not feasible?

@jlstevens

This comment has been minimized.

Copy link
Contributor

commented Apr 10, 2019

I also vote for keeping the API as consistent as possible. Why can't PolyDraw support the common style options for polygons which can then be set with styles or constants?

@philippjfr

This comment has been minimized.

Copy link
Contributor Author

commented Apr 10, 2019

Actually I vote for just respecting the usual style options set in the usual way and cycling if those are cycles, but I assume that's not feasible?

That's not consistent with the way cycles behave in HoloViews, styles cycle in an overlay but not within a single Polygons/Paths element.

Why can't PolyDraw support the common style options for polygons which can then be set with styles or constants?

I'm not following what your suggesting here, that's what the styles suggestion is no?

@jbednar

This comment has been minimized.

Copy link
Contributor

commented Apr 10, 2019

styles cycle in an overlay but not within a single Polygons/Paths element.

Ah, that's the key issue here. Worth brainstorming if there is a way more general than this approach to achieve within-Element cycling.

@philippjfr

This comment has been minimized.

Copy link
Contributor Author

commented Apr 10, 2019

if there is a way more general than this approach to achieve within-Element cycling.

That can now be done via dim style mappings but I don't see any way those could feasibly be implemented client-side.

@philippjfr philippjfr modified the milestones: v1.12.1, v1.12.2 Apr 22, 2019

@philippjfr

This comment has been minimized.

Copy link
Contributor Author

commented Apr 28, 2019

I'm going to go ahead and rename it to styles and support scalar values. Respecting cycles within an element would be highly inconsistent so I vote against that.

@philippjfr philippjfr force-pushed the style_cycling_draw_tools branch from 62db0b1 to 4d11138 Apr 28, 2019

@philippjfr philippjfr merged commit 9bf1270 into master Apr 28, 2019

4 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.05%) to 87.759%
Details

@philippjfr philippjfr deleted the style_cycling_draw_tools branch Apr 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.