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

Prop Descriptions #52

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

hipstersmoothie
Copy link

@hipstersmoothie hipstersmoothie commented Mar 27, 2019

closes #37

I lifted the code mostly from code mirror addon xml

  • Prop Type: descriptions
  • Prop Type: required
  • Prop Type: default
  • Prop Type: type
  • Component: description
  • Test on vanilla JS

Screen Shot 2019-03-28 at 8 45 11 PM

@hipstersmoothie hipstersmoothie changed the title [WIP] Descriptions Prop Descriptions Mar 29, 2019
@hipstersmoothie
Copy link
Author

@markdalgleish this should be all good now

@markdalgleish
Copy link
Member

Any chance you could add an example that exercises this? Right now, it's hard for me to verify this is working as desired.

@@ -0,0 +1,306 @@
/* eslint-disable new-cap */
Copy link
Member

Choose a reason for hiding this comment

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

Is this file borrowed from somewhere else? Seems like a copy-and-paste.

Copy link
Author

Choose a reason for hiding this comment

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

It was the addon-xml 😅 I went back for a second look and was able to use that code once it was registered to codemirror and just handle the tooltip stuff myself.

I switched to rendering the tooltip with lit-html. I can remove that and switch back to document methods if you want, but I think the code is a little clearer this way.

Copy link
Author

Choose a reason for hiding this comment

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

Actually I realized i could just use react and it become much clearer 👍

@hipstersmoothie
Copy link
Author

Any chance you could add an example that exercises this? Right now, it's hard for me to verify this is working as desired.

Added descriptions and defaults to the typescript example.

@hipstersmoothie hipstersmoothie mentioned this pull request Mar 30, 2019
@hipstersmoothie
Copy link
Author

hipstersmoothie commented Apr 5, 2019

@markdalgleish Does your major re-architecture effect any of this? I'm happy to change what ever else you need me to. Really excited to get this and #55 in.

EDIT: Doesn't look like our changes conflict much. I mostly just edited playroom.js

@hipstersmoothie
Copy link
Author

If these changes are too much a plugin API for suggestions would also probably fit my use case.

As it is playroom is only useful for people that are super familiar with the components and how they work together or have the docs up in a separate tab. Even when i try to use playroom myself I feel crippled without having the type definitions helping me along.

@yceballost
Copy link
Contributor

would be nice to merge this feature <3

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.

Prop Type Descriptions
3 participants