-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Allow embedding of GitLab snippets #6217
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
Allow embedding of GitLab snippets #6217
Conversation
Signed-off-by: Andrew Smith <espadav8@gmail.com>
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.
Great, this all seems quite reasonable
Thanks @tommoor 🙂 |
After merging I tried with this random snippet from GitLab and the result isn't too great as it ends up with a margin and scrollable, not like your screenshot at all. |
Have you got a screenshot of how it looks for you? The embed should be scrollable, since they can be long, and without the scrolling you can't see most of the snippet (unless we change the height of the iframe considerably). I think the margin for the iframe body is removed, but the GitLab code adds a bit of further margin or padding. I tried removing this, but with it gone it looks very cramped in the iframe. |
@tommoor Ah. That does look and function how I built it. What would your recommendation be to fix this then? I think without the scrolling it wouldn't really that useful since for nearly all snippets you won't be able to see enough of them to be useful withing a documentation page. |
I wonder if instead of iframing the embed script and can just embed the script tag and have the snippet expand as large as needed within the document itself. |
In hindsight I see that in the screenshots now. The GitHub embed is a fixed height and allows scrolling of the content while keeping the border and heading in the same place – the GitLab snippet cannot work like this? I'd rather not be injecting scripts into the main page if possible as it becomes a security risk. |
@tommoor Just tried embedding this Gist - https://gist.github.com/trpubz/e61ef0537ed6ea7b4c8fba9f649ccb22 - and I don't see any scrolling of that content locally. Is there something I need to change somewhere to enable the scrolling? |
Ah, I tried a different Gist - https://gist.github.com/ASkyeye/1e92771f287b0c6a8977edd348c5a1b1 - and that does have scrolling. Not sure why one does and the other doesn't. ... Trying another Gist with 2 files - https://gist.github.com/user2023cpu/48601160a9626f1d475af806f131d23d - and the embed isn't really useful. You can only see 2 lines of the second file (and I'm guessing if there were more than 2 files you wouldn't see any of the other files). |
I don't think it's ever been tested with these ones that have multiple files in a single gist. It certainly doesn't look very good either – at least there's not a bunch of extra margin? Ideally they would show the right height upto some reasonable upper limit? |
Removing the margin is straightforward with something like this (can open an MR if you'd like) diff --git a/shared/editor/embeds/GitLabSnippet.tsx b/shared/editor/embeds/GitLabSnippet.tsx
index 05bb38883..50f2585a4 100644
--- a/shared/editor/embeds/GitLabSnippet.tsx
+++ b/shared/editor/embeds/GitLabSnippet.tsx
@@ -13,7 +13,8 @@ function GitLabSnippet(props: Props) {
const id = snippetUrl.pathname.split("/").pop();
const snippetLink = `${snippetUrl}.js`;
const snippetScript = `<script type="text/javascript" src="${snippetLink}"></script>`;
- const styles = "<style>body { margin: 0; }</style>";
+ const styles =
+ "<style>body, body .gitlab-embed-snippets { margin: 0; }</style>";
const iframeHtml = `<html><head><base target="_parent">${styles}</head><body>${snippetScript}</body></html>`;
return ( There are some JS hacks around that will resize the iframe based on its content - for example https://stackoverflow.com/questions/819416/adjust-width-and-height-of-iframe-to-fit-with-content-in-it - but not sure how best to implement this within the application. I think if the auto-resizing iframe has some max-height where it stops, the issue would still happen at some point, unless scrolling is allowed past that max-height. There are some really long Gists out there - https://gist.github.com/choco-bot/4d2d06a7d6112822ff477ae0491b8575 - and when that is embedded all you can see right now are the first 6 bullet points. Testing that long Gist embed on Notion, and it looks like they might run the embed directly within the page and wrap the content there, since you get the full length of the Gist shown as a block. |
…30dfb (#15817) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [outlinewiki/outline](https://togithub.com/outline/outline) | minor | `0.73.1` -> `0.74.0` | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>outline/outline (outlinewiki/outline)</summary> ### [`v0.74.0`](https://togithub.com/outline/outline/releases/tag/v0.74.0) [Compare Source](https://togithub.com/outline/outline/compare/v0.73.1...v0.74.0) **Note:** Minimum Node version changed to 18 – if using Docker installation this is handled for you. ##### Improvements - Clicking on a link while editing will now automatically select the entire link - Mermaid diagrams are now rendered in HTML exports in [https://github.com/outline/outline/issues/6205](https://togithub.com/outline/outline/issues/6205) - Added "Share" option to document menu on mobile - New translations - Updated to Node 20 LTS - \[API] `comments.list` endpoint can now be used without a document ID ##### Fixes - Find and replace dialog no longer gets stuck without replace button - Fixed corruption in imports in [https://github.com/outline/outline/pull/6162](https://togithub.com/outline/outline/pull/6162) - Fixed recurring crash when DataDog and Sentry are enabled. - Fixed display of video nodes in document history - Fixed members can now create templates again - Fixed viewing public shares failed if user has an expired token in [https://github.com/outline/outline/pull/6134](https://togithub.com/outline/outline/pull/6134) - Restored ability to comment in code blocks in [https://github.com/outline/outline/issues/6154](https://togithub.com/outline/outline/issues/6154) - Comment functionality should not show in toolbar for view-only users in [https://github.com/outline/outline/issues/6011](https://togithub.com/outline/outline/issues/6011) - Added support for tldraw snapshot links in [https://github.com/outline/outline/pull/6210](https://togithub.com/outline/outline/pull/6210) - Restored PWA install in recent Chrome versions - Fixed extra empty page in HTML exports in [https://github.com/outline/outline/issues/6205](https://togithub.com/outline/outline/issues/6205) - Emoji in template titles are now applied correctly in [https://github.com/outline/outline/issues/6169](https://togithub.com/outline/outline/issues/6169) - Many dependency updates #### New Contributors - [@​NextFire](https://togithub.com/NextFire) made their first contribution in [https://github.com/outline/outline/pull/6134](https://togithub.com/outline/outline/pull/6134) - [@​EspadaV8](https://togithub.com/EspadaV8) made their first contribution in [https://github.com/outline/outline/pull/6217](https://togithub.com/outline/outline/pull/6217) - [@​RayYH](https://togithub.com/RayYH) made their first contribution in [https://github.com/outline/outline/pull/6238](https://togithub.com/outline/outline/pull/6238) **Full Changelog**: outline/outline@v0.73.1...v0.74.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://togithub.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy44OC4yIiwidXBkYXRlZEluVmVyIjoiMzcuODguMiIsInRhcmdldEJyYW5jaCI6Im1hc3RlciJ9-->
This PR contains the following updates: | Package | Update | Change | |---|---|---| | [outlinewiki/outline](https://github.com/outline/outline) | minor | `0.73.1` -> `0.74.0` | --- ### Release Notes <details> <summary>outline/outline (outlinewiki/outline)</summary> ### [`v0.74.0`](https://github.com/outline/outline/releases/tag/v0 8EF6 .74.0) [Compare Source](outline/outline@v0.73.1...v0.74.0) **Note:** Minimum Node version changed to 18 – if using Docker installation this is handled for you. ##### Improvements - Clicking on a link while editing will now automatically select the entire link - Mermaid diagrams are now rendered in HTML exports in outline/outline#6205 - Added "Share" option to document menu on mobile - New translations - Updated to Node 20 LTS - \[API] `comments.list` endpoint can now be used without a document ID ##### Fixes - Find and replace dialog no longer gets stuck without replace button - Fixed corruption in imports in outline/outline#6162 - Fixed recurring [OOM crash](https://twitter.com/tommoor/status/1730985110806184265) when DataDog and Sentry are enabled. - Fixed display of video nodes in document history - Fixed members can now create templates again - Fixed viewing public shares failed if user has an expired token in outline/outline#6134 - Restored ability to comment in code blocks in outline/outline#6154 - Comment functionality should not show in toolbar for view-only users in outline/outline#6011 - Added support for tldraw snapshot links in outline/outline#6210 - Restored PWA install in recent Chrome versions - Fixed extra empty page in HTML exports in outline/outline#6205 - Emoji in template titles are now applied correctly in outline/outline#6169 - Many dependency updates #### New Contributors - [@​NextFire](https://github.com/NextFire) made their first contribution in outline/outline#6134 - [@​EspadaV8](https://github.com/EspadaV8) made their first contribution in outline/outline#6217 - [@​RayYH](https://github.com/RayYH) made their first contribution in outline/outline#6238 **Full Changelog**: outline/outline@v0.73.1...v0.74.0 </details> --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy44Ny4yIiwidXBkYXRlZEluVmVyIjoiMzcuODcuMiIsInRhcmdldEJyYW5jaCI6Im1hc3RlciJ9--> Co-authored-by: Renovate Bot <bot@renovateapp.com> Reviewed-on: https://gitea.bui.services/calvinbui/ansible-monorepo/pulls/2031 Co-authored-by: renovate <renovate@noreply.gitea.bui.services> Co-committed-by: renovate <renovate@noreply.gitea.bui.services>
Similar to embedding a GitHub Gist, this allows embedding of a GitLab snippet. Supports embedding of personal snippets and snippets belonging to a group/project.