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

Unfancify #12

Merged
merged 17 commits into from
Nov 8, 2018
Merged

Unfancify #12

merged 17 commits into from
Nov 8, 2018

Conversation

andydandy74
Copy link
Collaborator

Purpose

This is the maximum of what we'll do during our session.
Most likely we will not get through all the functionality.

Before execution:
grafik
After execution:
grafik

FYI

@radumg
@teocomi
@wynged
@LongNguyenP

@andydandy74 andydandy74 self-assigned this Nov 5, 2018
@andydandy74 andydandy74 changed the title Unfancify dev Unfancify Nov 5, 2018
@teocomi
Copy link
Collaborator

teocomi commented Nov 7, 2018

awesome stuff!

Copy link
Member

@johnpierson johnpierson left a comment

Choose a reason for hiding this comment

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

Looking at this with my limited experience with extensions, it looks awesome to me. I am looking forward to this lab and am thrilled to be helping out. Not sure who needs to merge this, but I like it! 👍

@@ -0,0 +1,1300 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

Interestingly enough, I downloaded this file and all the node descriptions swapped languages except for the string inputs that were placed previously. I don't think it is a huge deal, but it was an interesting thing I stumbled upon.

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@johnpierson Could you try resaving it in your English language Sandbox and see if the descriptions are updated in English? If so, could you commit the changed version?

Copy link
Member

Choose a reason for hiding this comment

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

Even after resaving, it staus. But just for the string node. That is a little odd.

Copy link
Collaborator Author

@andydandy74 andydandy74 Nov 8, 2018

Choose a reason for hiding this comment

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

@johnpierson Let's try to reproduce that in Vegas. Smells like a bug.

Copy link
Collaborator

Choose a reason for hiding this comment

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

what does the actual json of the DYN file say ?

Copy link
Member

@johnpierson johnpierson Nov 8, 2018

Choose a reason for hiding this comment

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

After saving on my machine it looks like this. It's odd because string is the only node that does it.
image

Copy link
Member

Choose a reason for hiding this comment

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

This looks like a bug - serializing the description in localized language is fine, but it should not deserialize it from here anyway - I don't think

Copy link
Member

Choose a reason for hiding this comment

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

@johnpierson can you verify if this is also an issue for other base types like double or slider?

Copy link
Member

@johnpierson johnpierson Nov 8, 2018

Choose a reason for hiding this comment

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

@mjkkirschner For the sliders, it is working as expected. Need to find a graph with a double.

Copy link
Member

Choose a reason for hiding this comment

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

@johnpierson can you also clarify - do new nodes have the correct english description?

@@ -0,0 +1,4 @@
<ViewExtensionDefinition>
Copy link
Member

Choose a reason for hiding this comment

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

Loving the name BTW. We need to make a logo for it I think. :) One of the really fun parts of being a Dynamo developer is branding this stuff.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@johnpierson Got an idea?

Copy link
Member

@johnpierson johnpierson Nov 8, 2018

Choose a reason for hiding this comment

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

Two quick options I thought of. 😁
image

Copy link
Collaborator

Choose a reason for hiding this comment

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

What about a sheen of paper being rolled back - where 'fancify' is fancy and the 'un' is not... like steel, or rusty, or jagged.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I quite like the one with the bow-tie :-)

Copy link
Member

Choose a reason for hiding this comment

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

Here is the file if you were interested. (I renamed the extension to JPG from SVG for it to upload)
unfancify

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks @johnpierson

Copy link
Collaborator

@radumg radumg left a comment

Choose a reason for hiding this comment

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

👏

@andydandy74 andydandy74 merged commit f07ac6e into master Nov 8, 2018
@andydandy74 andydandy74 deleted the Unfancify_dev branch November 8, 2018 21:14
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.

6 participants