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

Rename Symbol and PlacedSymbol to something else, now that it clashes? #770

Closed
lehni opened this issue Sep 6, 2015 · 29 comments
Closed

Comments

@lehni
Copy link
Member

lehni commented Sep 6, 2015

Reason:
https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Symbol

Possible new names:

  • Clip
  • Definition
  • MovieClip
@lehni
Copy link
Member Author

lehni commented Sep 6, 2015

@puckey what do you think?

@lehni lehni changed the title Rename Symbol and PlacedSymbol to something else, now that it clashes? Rename Symbol and PlacedSymbol to something else, now that it clashes? Sep 6, 2015
@puckey
Copy link
Member

puckey commented Sep 6, 2015

How about:
Clip/PlacedClip?
Graphic/PlacedGraphic?

@iconexperience
Copy link
Contributor

Just an idea, because a symbol wraps an item:

ItemSymbol / PlacedItemSymbol

@iconexperience
Copy link
Contributor

Or maybe:
SymbolTemplate/PlacedSymbol

Or (hopefully my last suggestion):
ItemDef/PlacedItemDef

@lehni
Copy link
Member Author

lehni commented Jan 11, 2016

@iconexperience the first are quite long... And I think we should refrain from abbreviations in class names.

@puckey: the funny thing about Clip that I just realized is that if you don't use PaperScript, you'll end up using it like this:

paper.Clip

Maybe we should just call it Clippyand be done with it?

clippy

Some more to consider:

  • SymbolDefinition / PlacedSymbol
  • ClipDefinition / ClipItem
  • ClipDefinition / PlacedClip

J

@bmacnaughton
Copy link
Contributor

Maybe

MetaItem and MetaItemInstance or MetaInstance
AbstractSymbol and PlacedSymbol

Keeping Symbol in the name provides some continuity with the previous implementation but also some overloading of ES6 Symbol().

@lehni
Copy link
Member Author

lehni commented Jan 11, 2016

I don't think it's important to keep the reference to Symbol necessarily. We can announce the change in the change-log : )

MetaItem feels quite confusing to me, as meta can mean too many things, many of them quite, erm, meta.

And an AbstractSymbol wouldn't actually be abstract. It's just not placed yet?

@bmacnaughton
Copy link
Contributor

All fair concerns. I'm trying to widen the options as nothing really seems to resonate with all.

TemplateItem and TemplateInstance?

@lehni
Copy link
Member Author

lehni commented Jan 11, 2016

Is ClipDefinition / PlacedClip or ClipDefinition / ClipItem not good?

@lehni
Copy link
Member Author

lehni commented Jan 11, 2016

TemplateItem is misleading, since it won't be inheriting from Item, but TemplateInstance will.

@bmacnaughton
Copy link
Contributor

I'm thinking from the end-user perspective, so it seems like a TemplateItem to me. I guess some of it depends on whether the naming is for the developers or the end-users.

Clip makes me think of ClipArt or Video Clips. It ultimately can work, but it doesn't bring its purpose to mind when I read it.

@lehni
Copy link
Member Author

lehni commented Jan 11, 2016

But a template is something you start with and change, while clip art is something you cut and paste... I think that's more like it, actually?

The problem with TemplateItem is just that we use the term item for objects existing in the scene graph (and hence inheriting from Item) all across the documentation, and it would be really confusing to then have something tat's outside of the scene graph being called an item, wether somebody is a developer or not (anybody working with paper.js is a developer, not just an end user)

In Flash, i tused to be called a MovieClip. But then it had a timeline.

In SVG they are wrapped by a defs element: https://www.w3.org/TR/SVG/struct.html#DefsElement

@bmacnaughton
Copy link
Contributor

When I said end-user I meant as a developer-end-user of the library. The web browser end-user won't see any of this.

I see it as more of a template or a prototype - it's not actually cut and pasted, it's instantiated with specific positions. (At least that's how I have viewed them as a user of the library.) It also seems to me like a shared library, like a .DLL or a .SO file.

I have no experience whatsoever with Flash, so if that is a major concern I can't weigh in at all. I think the SVG defs idea makes sense - it's a definition, much like a C++ template is a definition.

SharedItem and SharedInstance?

I guess the only one the library user is really going to see is the first, i.e., the one that replaces Symbol.

@bmacnaughton
Copy link
Contributor

I think I've added about all I can. Whatever you decide will go in the change log :)

@iconexperience
Copy link
Contributor

Does this help from https://developer.mozilla.org/en-US/docs/Web/SVG/Element/symbol ?

"The symbol element is used to define graphical template objects "

TemplateObject?

@iconexperience
Copy link
Contributor

I think I like SymbolDefinition / PlacedSymbol best.

@bmacnaughton
Copy link
Contributor

The sketch parser already warns that I should not use Symbol() as a constructor.

@lehni
Copy link
Member Author

lehni commented Jan 11, 2016

@iconexperience I also think that's the best so far, and the closest to what we currently have. SymbolDefinition is just a bit long, which is why I also liked ClipDefinition / PlacedClip. Then there is also the question about the SymbolDefinition#definition property... It could actually be called #item, as that's what it is, really. (keeping the old #definition as an alias to #item around for quite a while, of course). What do you think?

@bmacnaughton oh wow, I wasn't aware of it, but you're right!

@lehni
Copy link
Member Author

lehni commented Jan 12, 2016

So here's the decision after a Skype session with @puckey about this:

  • SymbolSymbolDefintion
  • PlacedSymbolSymbolItem
  • Symbol#definitionSymbolDefintion#item

var path = new Path.Circle({
    center: [0, 0],
    radius: 100,
    strokeColor: 'red'
});
var symbolItem = new SymbolDefinition(path).place([100, 00]);
var symbolDefintion = project.symbols[0];
console.log(symbolDefintion.item === path); // true

Sounds good?

@iconexperience
Copy link
Contributor

Sounds very good to me.

@lehni
Copy link
Member Author

lehni commented Jan 12, 2016

We were considering this, but it's just oh so very long:

var symbolDefintion = project.symbolDefinitions[0];

@iconexperience
Copy link
Contributor

Not much longer than getTangentAtTime() :)

@lehni
Copy link
Member Author

lehni commented Jan 12, 2016

Hahaha true! Is that too long, too?

@iconexperience
Copy link
Contributor

I see all the logic behind that and it will be easier to learn, but of course I have gotten used to the old, shorter names. I did not even know until recently that t means curvetime. If I had to choose, I would probably use getTangentAtT(), but I would not expect that everybody will be happy with that.

@lehni
Copy link
Member Author

lehni commented Jan 12, 2016

And getTangentAtPt() then too?

@lehni
Copy link
Member Author

lehni commented Jan 12, 2016

(BTW, this is the wrong place for that discussion)

@iconexperience
Copy link
Contributor

I have answer this at #563 (comment)

@lehni lehni added this to the v0.10.0 milestone Jan 13, 2016
@lehni
Copy link
Member Author

lehni commented Jan 15, 2016

I am thinking of also removing Project#symbols. We don't have Project#gradients either, and nobody ever complained about that. And I am weary of soon having to implement a Project#patterns. The only use case for this I can think of is if people want to create a graphics editor and have access to the symbol definitions of the document for the symbols that aren't actually placed... The one that are placed can be found through #getItems({ class: SymbolItem }), and from there it is easy to get the symbol definition.

But I think it's wrong if our API tries to address this use case. What we could do instead is offer the #data property on Project and persist it there as well. People can then use this to store all kind of information along with their files.

And SVGImport would require a way to give access to the dictionary on import, which holds these definitions. This could be added to the onLoad callback as a second argument, for example.

@puckey, @iconexperience: what do you think about this suggestion?

@lehni
Copy link
Member Author

lehni commented Jan 15, 2016

One more thing: The current PlacedSymbol#symbol should probably become SymbolItem#definition.

But is this then confusing?

var path = new Path.Circle({
    center: [0, 0],
    radius: 100,
    strokeColor: 'red'
});
var symbolDefinition = new SymbolDefinition(path);
var symbolItem = symbolDefinition.place([100, 00]);
console.log(symbolItem.definition == symbolDefintion); // true
console.log(symbolDefintion.item == path); // true, as it points to the actual definition
console.log(symbolDefintion.item != symbolItem); // true, because it's not pointing to placed items

@lehni lehni closed this as completed in bc27296 Jan 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants