-
Notifications
You must be signed in to change notification settings - Fork 18
feat(directive forest): create copy to clipboard button for properties view #38
Conversation
@@ -103,5 +106,18 @@ export class DirectiveExplorerComponent implements OnInit { | |||
}); | |||
return result; | |||
} | |||
|
|||
copy(dataToCopy: object): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We copy the entire descriptor this way, right? We don't want metadata in the users' clipboards.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll post a demo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we should transform the object to preserve only the values, not the metadata that we send from the backend. Here's what I copied from the zippy:
{"visible":{"type":6,"value":false,"editable":true,"expandable":false,"preview":"false"},"title":{"type":1,"value":"► Click to expand","editable":true,"expandable":false,"preview":"\"► Click to expand\""}}
Notice the properties type
, preview,
editable`, etc. these shouldn't be part of the payload that ends up in the user's clipboard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reread your comment a moment ago and realized what you meant, my apologies haha. I'm thinking I can somehow take advantage of the existing serializer functions in state-serializer.ts, maybe with an options argument on whether to include the meta-data or not. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After examining state-serializer.ts
I don't think it would be clean to hijack the existing serialize functions. I have an idea of how to implement this but since this feature isn't on the main issues list, I'll put it on hold for now and give it a shot this weekend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A function which transforms the object:
{"visible":{"type":6,"value":false,"editable":true,"expandable":false,"preview":"false"},"title":{"type":1,"value":"► Click to expand","editable":true,"expandable":false,"preview":"\"► Click to expand\""}}
to:
{
"visible": false,
"title": "► Click to expand"
}
Should be fine. You can just strip few properties. With nested props, we can set as value the preview.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice feature! Left a single comment. Would you update the commit message so the commit type is lowercase?
30162b4
to
c830399
Compare
c830399
to
65ab968
Compare
65ab968
to
a7f5396
Compare
document.execCommand('copy'); | ||
} | ||
|
||
private _cleanPropData(treeData: Properties, restructuredTreeData = {}): object { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't have to be a private method. A utility function will work just fine and won't pollute the prototype.
…s view wrapper add snack bar notification
a7f5396
to
70eb526
Compare
…s view wrapper (rangle#38) add snack bar notification
…rapper (rangle#38) add snack bar notification
…s view wrapper (rangle#38) add snack bar notification
A small nice-to-have.
Limitation: Cannot copy nested properties.
Edit: Underestimated the scope of this feature. Putting it on hold to focus on main issues. Will tackle this one in my spare time.