add local test file and support html parsing #8
Conversation
| } | ||
|
|
||
| const DefaultConfig: Config = { |
There was a problem hiding this comment.
i moved these into individual defaults because otherwise you wouldn't get the proper default if you passed in a partial config
src/index.ts
Outdated
| } | ||
|
|
||
| case TextMode.Html: | ||
| const newNode = document.createElement("span"); |
There was a problem hiding this comment.
by default wrapped this in a span 🤷 I guess could be an additional customization to specify what the default wrapping element is too
There was a problem hiding this comment.
I think it's a bit weird to enforce a wrapping element in the input which is why I added this in the first place (i.e. i want to pass in - hello <b>with bold</b> rather than having to do - <span> I'm wrapping my text in a span </span>
There was a problem hiding this comment.
wish we had fragments in regular HTML 😔 looks like span can only have these as children: https://developer.mozilla.org/en-US/docs/Web/Guide/HTML/Content_categories#phrasing_content
this seems comprehensive enough for now? thoughts on adding an HTML class to this span so people can style it?
There was a problem hiding this comment.
yeah it's not ideal :( I think ideally we'd try to interpret it as HTML and then fall back to a default element (that defaults to span but could be configured) if not present (so then you could put whatever you wanted as the wrapper) but i wasn't sure how to do that optimistic determinatio well.
re: class hmm i think we should avoid adding new classes as much as possible and have them style via targeted styling i.e. #telescope span...
There was a problem hiding this comment.
i can update with being able to specify a default element tag
| @@ -105,30 +151,22 @@ function _hydrate(line: Content, node: any, shouldExpandOnMouseOver: boolean = f | |||
| } | |||
| } else { | |||
| // otherwise, this is a leaf node | |||
| if (lineText.includes("@Q ")) { | |||
There was a problem hiding this comment.
this got moved up into the helper function. Side effect here is that it doesn't just act on leaf nodes anymore
lib/index.d.ts
Outdated
| } | ||
| declare const DefaultConfig: Config; | ||
| declare type CreateTelescopicTextConfig = Pick<Config, "shouldExpandOnMouseOver" | "textMode">; |
src/index.ts
Outdated
| } | ||
|
|
||
| case TextMode.Html: | ||
| const newNode = document.createElement("span"); |
There was a problem hiding this comment.
wish we had fragments in regular HTML 😔 looks like span can only have these as children: https://developer.mozilla.org/en-US/docs/Web/Guide/HTML/Content_categories#phrasing_content
this seems comprehensive enough for now? thoughts on adding an HTML class to this span so people can style it?
| ` | ||
| const config = { textMode: TextMode.Html }; | ||
| const poemContent = createTelescopicTextFromBulletedList(content, config); | ||
| ``` | ||
|
|
There was a problem hiding this comment.
might be good to have a disclaimer somewhere about potential dangers of using TextMode.Html as it does a .innerHTML under the hood
There was a problem hiding this comment.
ah yeah sure i can add in the readme / docs. Theoretically should be fine unless you're using someone elses code haha
Co-authored-by: Jacky Zhao <j.zhao2k19@gmail.com>
|
@jackyzha0 updated! |
|
🔥 |
Add
textModeparameter which hastextas the default param and uses normal behavior and offershtmlfor custom HTML to give control to the end-user on how to render things and interact.Also added a
local-test.htmlfile for local testing that is set up to pull from the locally built lib. Imagining we can add a list of both examples and test cases for new functionality thereScreen.Recording.2022-03-05.at.1.36.12.PM.mov