-
Notifications
You must be signed in to change notification settings - Fork 5
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
Prototype embed blocks #602
Conversation
✅ Deploy Preview for veda-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
@nerik This is definitely a great enhancement to shorten the gap between dashboard and other tools.
I think maybe we should maintain the block/figure structure to have more control and consistency. This would result in a behavior like the Map
. What do you think?
<Block>
<Figure>
<Embed />
</Figure>
</Block>
I also tried very quickly to add a window frame (<BrowserFrame>
component) to contain the embed, and I think it looks kind of nice. Thoughts?
@danielfdsilva Excellent suggestions, thanks 👍 While using a Thoughts @ricardoduplos ? |
@nerik I like that idea. Maybe instead of making the url fully visible, we can just offer a way to open in a new tab? Otherwise it may look to much like a browser inside a page. |
Wah it looks so nice 👏 and this will encourage editors to bring in their own content. But I have an open-ended question about how much content should go to iframe vs staying in mdx. I somehow thought iframe would be strictly about the map, and all other text-based contents will be in mdx. So in the fire example discovery, the text contents such as
will be in mdx file, not in iframed contents. I was thinking in this way so editors don't need to think about making the fonts rights etc. This also means that we eventually need to be 'the editorial desk' to give suggestions for editing iframe contents... which I am unsure is a good idea. 🤔 |
a92f001
to
6662edb
Compare
@hanbyul-here Thanks. Good catch. We discussed with @jsignell and she removed the text content from the iframe, which I integrated into the story itself - see NASA-IMPACT/veda-config#295 @danielfdsilva Also added a link to open in a new tab. |
already reviewed by @hanbyul-here
Adds an
<Embed />
block that instantiate an iframe for the use case described here.