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

feat(Control) Add control fill/stroke options #7524

Closed

Conversation

rockerBOO
Copy link
Contributor

This allows custom controls to have specific fill/stroke outside specific
object options (fabric.Object.cornerStrokeStyle).

const object = new fabric.Rect({ width: 5, height: 10 })
object.set('controls', {
 mtr: new fabric.Control({ fill: 'red', stroke: 'blue' })
})

This allows custom controls to have specific fill/stroke outside specific
object options (fabric.Object.cornerStrokeStyle).
stroke = !transparentCorners && (styleOverride.cornerStrokeColor || fabricObject.cornerStrokeColor),
stroke = !transparentCorners && (
styleOverride.cornerStrokeColor || fabricObject.cornerStrokeColor || this.stroke
),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this gets hairy.
How do we decide priority?
ideally the most specific part always win.
so do i have to paint the stroke?

well if !transparentCoreners
if this stroke is not undefined and is not defined as transparent or not defined as empty
if styleOverride.cornerStrokeColor is not undefined and is not defined as transparent or not defined as empty
if ....

:(

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the complexity but not sure of how to cascade options like this without it?

transparentCorners, stroke = false,
!transparentCorners
  > styleOverride
  > fabricObject
  > stroke

If styleOverride is set, it will override all other options
If fabricObject is set (object specific options for controls should take priority)
If stroke is set on the control (control settings are the default option and will fall through in priority until the last option.)

I can separate it into 2 statements for the requested color and if the corners are transparent.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what take precedence is a bit a matter of opinion, the important is that we document it.
Controls are generally shared between objects, this is how the library is setup by default, so it makes sense that object settings take precedence.
I m not sure what is the default value of those properties, because if is not undefined it becomes complicated.
Maybe we should consider the empty space a lazy default value, while if you want to take the precedence with a transparent one, instead of using false or empty space you use the word transparent

@stale
Copy link

stale bot commented Dec 21, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Issue marked as stale by the stale bot label Dec 21, 2021
@stale stale bot closed this Dec 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Issue marked as stale by the stale bot
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants