Skip to content

Conversation

@StefRave
Copy link
Contributor

…like styling

Removed the dependency on the Daniel font. The font can be set using style sheets. However
this introduced a requirement to delay rendering until the font is loaded completely so that
text size can be calculated accurately.

…like styling

Removed the dependency on the Daniel font. The font can be set using style sheets. However
this introduced a requirement to delay rendering until the font is loaded completely so that
text size can be calculated accurately.
@bramp
Copy link
Owner

bramp commented Aug 10, 2015

Very nice! I quickly scanned the change, but going to wait until the weekend to fully review/pull.

I fully want to do this migration, but some quick comments/concerns (off the top of my head)

  1. I would like to make the loading of the hand drawn font seamless. So no work for the user other than having the font in the right path.
  2. Is it possible to put your snap.svg work into a separate theme, so Rapheal and snap can coexist? I don't know what my migration plan is, but is it possible to offer both for a while?
  3. I will do a major version bump after I pull this change.

@bramp bramp added this to the SVG Only milestone Aug 10, 2015
@StefRave
Copy link
Contributor Author

No hurry, I don't have any time to do anything the next week

  1. I understand, and agree. I could not find a quick solution a short amount of time, but it should be possible.
  2. It is possible, but I am not sure if it is worth the hassle. Having to maintain Raphael & Snap.SVG in one codebase can be a challenge. Putting 1.x on a branch will keep code more clean and allow more innovation.
  3. I think a is a good solution. Maintain the Raphael version for a while and focus on new features for the SVG version. I think we can make upgrading to the new version easy, but I don't know how this will affect old browser support. Probably >=IE9 will be possible.

@schrepfler
Copy link

Nice effort @StefRave!

Copy link
Owner

Choose a reason for hiding this comment

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

The above three lines aren't needed. (they are copied and pasted from a demo)

@bramp
Copy link
Owner

bramp commented Aug 11, 2015

I've pulled your code into a new branch, and started hacking on it. There are some inconsitances with the old vs new rendering. I'm also refactoring this so there is a RaphaelTheme and a SnapTheme. It was quite easy to keep both, without duplicating code. This is mainly useful so I can easily compare old to new (to check for regressions). I'll update you shortly.

@bramp
Copy link
Owner

bramp commented Aug 14, 2015

I've pulled this into a new SVG branch: https://github.com/bramp/js-sequence-diagrams/tree/svg and started to heavily refactor to allow both Raphael and Snapsvg at the same (if you so wish). I had to cleanup some of this new snapsvg code to match the existing API correctly (for example, snapsvg assumed the container was a SVG, where previous Raphael accepted div/td/etc). Still a work in progress, but getting there. I will close this PR, and track the work in #127

@bramp bramp closed this Aug 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants