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

Added attributes to svg.Startview and svg.StartviewUnit #38

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

countcb
Copy link

@countcb countcb commented Jul 15, 2017

Added attributes to svg.Startview and svg.StartviewUnit

Was wondering why these two methods had no way of adding attributes like the others.
I hope the changes are ok. If not I can change some things or you can deny the pull request :)

@ajstarks
Copy link
Owner

StartRaw was meant to handle the arbitrary case.

@countcb
Copy link
Author

countcb commented Jul 17, 2017

Hmm. ok.
I found it a little bit strange that 3 out of 5 Start methods had the attributes param and only 2 didn't.

Start, Startunit and Startpercent already have the extra attributes.
Only Startview and StartviewUnit don't.

I feel that all of them should have them (no harm since the attributes param is optional), otherwise it feels a little bit inconsistent.

But feel free to close this PR if you think different :)

@ajstarks
Copy link
Owner

@countcb you are right, it is inconsistent, and my (probably) lame attempt at a general solution was StartRaw.

thinking...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants