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

Docs: Add StackBlitz edit button for examples #37233

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

julien-deramond
Copy link
Member

@julien-deramond julien-deramond commented Oct 1, 2022

Related issues

Closes #36857

Description

This PR suggests to add a StackBlitz edit button for each Bootstrap example.

TODO: add more details to explain the technical solution

Motivation & Context

TODO

Type of changes

  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the contributing guidelines
  • My code follows the code style of the project (using npm run lint)
  • My change introduces changes to the documentation
  • I have updated the documentation accordingly

Live previews

Manual tests

For each example, try the StackBlitz edit button and then check if the rendering is the same as within our docs: CSS, JS, etc.

Remaining things to do

  • Handle 404 errors when using fetch
  • Handle extra JS files
  • Handle RTL (CSS)
  • Handle RTL (JS)
  • Check how to handle relative URL like /docs/5.2/assets/brand/bootstrap-logo.svg in the body
  • Rebase this branch onto main when Rename examples CSS/JS files for consistency #37267 will be merged
  • If Fix accessibility of examples buttons #37498 is merged, propagate the same modification in this PR
  • Remove the bootstrap-bundle.min.js in `<script> for navbars example for example; it causes an issue where the burger menu can't be closed
  • Clean up the code
  • Remove inline style for the button
  • Check if StackBlitz can format the "uploaded" files (e.g. style.css)
  • Check all examples

@julien-deramond julien-deramond changed the title Add StackBlitz edit button for examples Docs: Add StackBlitz edit button for examples Oct 1, 2022
@julien-deramond
Copy link
Member Author

julien-deramond commented Oct 6, 2022

Before going further in the details of this PR, what do you think @twbs/js-review about this suggestion?

@GeoSot
Copy link
Member

GeoSot commented Oct 6, 2022

@julien-deramond it would help a lot if you are able to separate the renaming changes, and continue with the functionality additions,

@julien-deramond
Copy link
Member Author

julien-deramond commented Oct 6, 2022

@julien-deramond it would help a lot if you are able to separate the renaming changes, and continue with the functionality additions,

Yes, if you're OK with the renaming for the consistency (and it's a bit mandatory to have a generic approach for those buttons), I can create a separate PR right now and then work on the addition of the StackBlitz edit buttons.

@GeoSot
Copy link
Member

GeoSot commented Oct 6, 2022

At least for me, I prefer it, as it helps on files searching too

@julien-deramond julien-deramond force-pushed the main-jd-stackblitz-for-examples branch from 2e86ecd to f555231 Compare October 7, 2022 06:03
@GeoSot GeoSot force-pushed the main-jd-stackblitz-for-examples branch from f555231 to c35148b Compare October 8, 2022 00:01
@GeoSot GeoSot force-pushed the main-jd-stackblitz-for-examples branch from c35148b to 5267ef5 Compare October 12, 2022 21:47
@@ -9,6 +9,10 @@
<script src="https://cdn.jsdelivr.net/npm/@stackblitz/sdk@1/bundles/sdk.umd.js"></script>
{{- end }}

{{ if eq .Page.Layout "single" -}}
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this duplicated in another layout file?

If so, we should deduplicate common code because it's a significant amount of it. Ideally, the inline JS should go into external JS files, but it won't be easy if we need Hugo to process them.

So, at least deduplicating common StackBlitz layout code this would be an improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Edit examples on StackBlitz
3 participants