Allow popovers examples to be tried on StackBlitz #36127

julien-deramond · 2022-04-07T10:51:35Z

This PR tries to allow popovers examples to be tried on StackBlitz.

In the documentation, they need some JS available in site/assets/js/snippets.js.
IMHO it would be troubling to have all this file integrated in StackBlitz.
So in this PR I'm trying to:

Split site/assets/js/snippets.js into small reusable parts
Create a new javascript parameter for the example shortcode containing the name of the reusable JS file to embed into StackBlitz
Allow site/layouts/partials/scripts.html to gather this information and embed the content of the JS file in a <script>.

Sub-tasks

Instead of using <script>, try to create a real index.js file in StackBlitz imported by index.html.
~~Find a better way to share the JS name between the doc, the shortcode and the site/layouts/partials/scripts.html~~
Security concerns to gather some JS file content?

Live previews

Live demo
Four directions
Custom popovers—Not working because doesn't exist in 5.1.3
- That's a problem because we will only be able to suppose that the components will work on StackBlitz when we develop them. We will have to wait until it they are released in order to see if it is OK or not.
Dismiss on next click
Disabled elements

Not tackled here

Be able to pass multiple JS files (might be useful for other examples more complex)—we will see that while developing the fix for the other components
Be able to split the SCSS file used by examples and to pass it (them) like the JS files
(Optional) Hide some examples if too complex or impossible to try on StackBlitz

mdo · 2022-04-14T04:00:30Z

I actually don't mind this solution... feels actually quite direct?

julien-deramond · 2022-04-14T18:33:59Z

I actually don't mind this solution... feels actually quite direct?

Yep. I haven't found any other acceptable solutions atm so I'll continue with that atm.

In a separate commit (4e31bde) you'll find a version where the JS content is in a separate index.js file in StackBlitz.
For that, I simplified the configuration by saying that we always use the 'javascript' template (we could test if the content is empty or not and juggle with 'javascript' and 'html' templates if needed).
But to do that, I had to insert <script src="https://cdn.jsdelivr.net/npm/bootstrap@5.1.3/dist/js/bootstrap.bundle.min.js"></script> in the <head> contrary to what's explained in the Starter Template.

julien-deramond · 2022-04-16T09:10:58Z

Let's do it step by step. IMHO this PR can be reviewed as it is because it already offers an improvement for the users.
The description contains what's not done in this PR and could be tackled in other PRs in the same style while finishing step by step all the tasks tracked in #36091 (I could create a tracking issue instead of a PR if you prefer).

site/layouts/shortcodes/example.html

julien-deramond · 2022-04-16T09:41:52Z

site/content/docs/5.1/components/popovers.md

@@ -48,15 +48,15 @@ var popoverList = popoverTriggerList.map(function (popoverTriggerEl) {

 We use JavaScript similar to the snippet above to render the following live popover. Titles are set via `title` attribute and body content is set via `data-bs-content`.

-{{< example >}}
+{{< example javascript="popover-demo.js" >}}
 <button type="button" class="btn btn-lg btn-danger" data-bs-toggle="popover" title="Popover title" data-bs-content="And here's some amazing content. It's very engaging. Right?">Click to toggle popover</button>


Don't know why but the code contains title="Popover title", it works in the doc but I can't find the title in the DOM and so it is missing in StackBlitz as well 🤔 If I replace title="Popover title" by data-bs-title="Popover title", everything's working correctly but I don't know the impact of such a change.

Your answer is here unfortunately 😞

bootstrap/js/src/tooltip.js

Lines 513 to 525 in 51535cd

_fixTitle() {

const title = this._config.originalTitle

if (!title) {

return

}

if (!this._element.getAttribute('aria-label') && !this._element.textContent) {

this._element.setAttribute('aria-label', title)

}

this._element.removeAttribute('title')

}

OK so all popovers that contain titles won't be exportable in StackBlitz. StackBlitz button should be hidden for those cases. Except if someone thinks about a way of doing it differently.

This one is describing the title attribute fallback process, so it may is better to keep it as it was (no btn)

If we have other popovers using title, I think it's ok to change them using data-bs-title

XhmikosR · 2022-04-17T05:21:50Z

We should really look into using esbuild for docs JS and thus split application.js into smaller files properly imported.

site/layouts/shortcodes/example.html

julien-deramond · 2022-04-24T19:12:49Z

We should really look into using esbuild for docs JS and thus split application.js into smaller files properly imported.

Is it mandatory for this PR (and the nexts to come related to fix StackBlitz example) or can it be treated separately?

mdo · 2022-04-24T21:38:37Z

Definitely separately :)

GeoSot · 2022-05-03T09:40:22Z

@julien-deramond Do you have any alternative in mind, to avoid this (using a link or something else)?

If we are in luck of alternatives, I suppose we can live with it

julien-deramond · 2022-05-03T10:19:11Z

@julien-deramond Do you have any alternative in mind, to avoid this (using a link or something else)?

The only alternative I'm thinking of (not tested so maybe it is dumb) would be to do something like that:

Have site/assets/js/popover-demo.js accessible from https://getbootstrap.com/docs/5.1/assets/js/ (and not only within docs.min.js).
Still call the shortcode with example js_file="popover-demo.js" but then having data-js-file="popover-demo.js" (and not the content of the file)
When clicking on the button:
- Request the JS file by concatenating {baseUrl}/docs/5.1/assets/js/ and popover-demo.js (now exported in assets/js)
- Read its content
- Export it as it is already done in this PR in StackBlitz.

If it works and is considered as a better solution please keep in mind that we would have several JS files used only for StackBlitz in assets/js; and then CSS files as well to fix the other issues with StackBlitz. It would also mean that for each Pull Request, we would have more files in Netlify and that can causes issues with some quotas you may have (the global package of the documentation would be heavier).

Do you want me to try this solution to see if it is feasible?

GeoSot · 2022-05-03T21:10:17Z

Do you want me to try this solution to see if it is feasible?

I am not the right person to give this answer, but it would come soon or late

\cc @mdo @XhmikosR

mdo · 2022-05-06T04:53:40Z

This will need a rebase or update to resolve a merge conflict :). We just merged a PR to split some more of the JS into separate files.

GeoSot · 2022-05-10T07:42:33Z

@julien-deramond in order to simplify a bit the whole process, can you investigate the case to make accessible & include by default the snippets.js?
If is is possible, we just need to add some clear comments about each functionality

julien-deramond · 2022-05-10T09:14:36Z

Haven't tried it explicitly but it is possible to always include the content of this file in the index.js in StackBlitz rather than using the split files.

I rather thought to split it in order to have only what's needed for each example. If you think in the core team that's not a problem for the users who will share between them those examples with extra JS (and then extra CSS for the broken use cases of StackBlitz due to missing CSS) I can modify this PR in order to do that. It won't fix only popovers but probably a lot of examples at the same time.

As you said, in this case snippets.js would need some clear comments for the user to understand what's useful and when.

Edit: I missed the "make accessible" part of your comment in my first answer. I'll try to see how it can be accessed directly from outside. But if we stick with a single file (not a split one) maybe that's not useful. Let's try.

julien-deramond · 2022-05-27T07:42:07Z

Closing this one superseded by #36352

julien-deramond mentioned this pull request Apr 7, 2022

[WIP] Hide or fix StackBlitz examples #36091

Closed

95 tasks

julien-deramond changed the title ~~[WIP] Allow popovers to be tested on StackBlitz~~ [WIP] Allow popovers examples to be tried on StackBlitz Apr 7, 2022

julien-deramond force-pushed the main-jd-stackblitz-and-popovers branch from fdad810 to cbc0566 Compare April 14, 2022 17:57

julien-deramond marked this pull request as ready for review April 16, 2022 09:11

julien-deramond requested a review from a team as a code owner April 16, 2022 09:11

julien-deramond commented Apr 16, 2022

View reviewed changes

site/layouts/shortcodes/example.html Outdated Show resolved Hide resolved

julien-deramond changed the title ~~[WIP] Allow popovers examples to be tried on StackBlitz~~ Allow popovers examples to be tried on StackBlitz Apr 16, 2022

julien-deramond commented Apr 16, 2022

View reviewed changes

XhmikosR reviewed Apr 17, 2022

View reviewed changes

site/layouts/shortcodes/example.html Outdated Show resolved Hide resolved

julien-deramond force-pushed the main-jd-stackblitz-and-popovers branch from bce7fb4 to 2aa3159 Compare April 19, 2022 17:14

mdo added docs js labels Apr 30, 2022

mdo added this to In progress in v5.2.0-stable via automation Apr 30, 2022

mdo added this to In progress in v5.2.0 via automation Apr 30, 2022

GeoSot force-pushed the main-jd-stackblitz-and-popovers branch from 2aa3159 to abc01b0 Compare May 3, 2022 09:09

Allow popovers to be displayed on StackBlitz

58986d3

julien-deramond force-pushed the main-jd-stackblitz-and-popovers branch from abc01b0 to 58986d3 Compare May 7, 2022 08:20

julien-deramond mentioned this pull request May 13, 2022

chore/merge-main@795167d Orange-OpenSource/Orange-Boosted-Bootstrap#1247

Merged

71 tasks

mdo removed this from In progress in v5.2.0 May 13, 2022

This was referenced May 14, 2022

Fix StackBlitz examples by embedding snippets.js when needed #36352

Merged

[Tracking] Fix StackBlitz examples #36391

Open

julien-deramond closed this May 27, 2022

v5.2.0-stable automation moved this from In progress to Done May 27, 2022

julien-deramond mentioned this pull request May 27, 2022

Fix StackBlitz Popovers and Tooltips examples #36449

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow popovers examples to be tried on StackBlitz #36127

Allow popovers examples to be tried on StackBlitz #36127

julien-deramond commented Apr 7, 2022 •

edited

mdo commented Apr 14, 2022

julien-deramond commented Apr 14, 2022 •

edited

julien-deramond commented Apr 16, 2022 •

edited

julien-deramond Apr 16, 2022

GeoSot May 3, 2022 •

edited

julien-deramond May 3, 2022 •

edited

GeoSot May 3, 2022

XhmikosR commented Apr 17, 2022

julien-deramond commented Apr 24, 2022

mdo commented Apr 24, 2022

GeoSot commented May 3, 2022

julien-deramond commented May 3, 2022

GeoSot commented May 3, 2022

mdo commented May 6, 2022

GeoSot commented May 10, 2022 •

edited

julien-deramond commented May 10, 2022 •

edited

julien-deramond commented May 27, 2022

	_fixTitle() {
	const title = this._config.originalTitle

	if (!title) {
	return
	}

	if (!this._element.getAttribute('aria-label') && !this._element.textContent) {
	this._element.setAttribute('aria-label', title)
	}

	this._element.removeAttribute('title')
	}

Allow popovers examples to be tried on StackBlitz #36127

Allow popovers examples to be tried on StackBlitz #36127

Conversation

julien-deramond commented Apr 7, 2022 • edited

Sub-tasks

Live previews

Not tackled here

mdo commented Apr 14, 2022

julien-deramond commented Apr 14, 2022 • edited

julien-deramond commented Apr 16, 2022 • edited

julien-deramond Apr 16, 2022

Choose a reason for hiding this comment

GeoSot May 3, 2022 • edited

Choose a reason for hiding this comment

julien-deramond May 3, 2022 • edited

Choose a reason for hiding this comment

GeoSot May 3, 2022

Choose a reason for hiding this comment

XhmikosR commented Apr 17, 2022

julien-deramond commented Apr 24, 2022

mdo commented Apr 24, 2022

GeoSot commented May 3, 2022

julien-deramond commented May 3, 2022

GeoSot commented May 3, 2022

mdo commented May 6, 2022

GeoSot commented May 10, 2022 • edited

julien-deramond commented May 10, 2022 • edited

julien-deramond commented May 27, 2022

julien-deramond commented Apr 7, 2022 •

edited

julien-deramond commented Apr 14, 2022 •

edited

julien-deramond commented Apr 16, 2022 •

edited

GeoSot May 3, 2022 •

edited

julien-deramond May 3, 2022 •

edited

GeoSot commented May 10, 2022 •

edited

julien-deramond commented May 10, 2022 •

edited