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

Examples: Introduce <skip> for our examples + Add some Stackblitz examples + undupe code + homogenize our design for the examples in the doc #38933

Closed
wants to merge 10 commits into from

Conversation

louismaximepiton
Copy link
Member

@louismaximepiton louismaximepiton commented Jul 19, 2023

Description

⚠️ This PR can be split into many if needed ⚠️
First part is #39328. Rebase once merged.

Rewrite examples shortcodes. -> #39328.
Homogenize the examples over the doc. -> #39328.
Add some missing use cases in docsref page. -> #39328.
Change the CSS for example. -> #39328.
Some proposals to better the doc, meaning having some more Stackblitz examples.
Avoid having some dupe code to maintain
Introduce the use of <skip> for example shortcode (not visible from anywhere for end-user). -> #39747.

More details about the files changed

Changes that happen in the doc

New working StackBlitz example

Still some issues on them

  • Carousel aren't initialized on StackBlitz but still work fine once initialized.
  • Scrollspy aren't initialized on StackBlitz but still work fine once initialized.

Motivation & Context

Homogenize handling of our examples
Propose some missing examples to Stackblitz

Type of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would change existing 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
  • I have added tests to cover my changes
  • All new and existing tests passed

Live previews

Some examples of implementation

Check that the rendering of this page look good.

Related issues

Might close #36391.

@julien-deramond
Copy link
Member

julien-deramond commented Jul 19, 2023

Haven't looked at it in detail, but love it so far! This is neat! More StackBlitz and less (and probably get rid of) <div class="bd-example">s.

@XhmikosR
Copy link
Member

Can you please fix your PR title and commits to be more descriptive please?

@louismaximepiton louismaximepiton changed the title Examples: Proposal for the doc examples Examples: Introduce <skip> for our examples + Add some Stackblitz examples + undupe code + homogenize our design for the examples in the doc Jul 24, 2023
@louismaximepiton louismaximepiton marked this pull request as ready for review July 24, 2023 14:17
@louismaximepiton louismaximepiton requested review from a team as code owners July 24, 2023 14:17
</div>
</div>
</div>
```
{{< /example >}}
Copy link
Member

Choose a reason for hiding this comment

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

indentation is off, there might be more places

@@ -109,7 +109,7 @@ You can replace the text within the `.navbar-brand` with an `<img>`.
<nav class="navbar bg-body-tertiary">
<div class="container">
<a class="navbar-brand" href="#">
<img src="/docs/{{< param docs_version >}}/assets/brand/bootstrap-logo.svg" alt="Bootstrap" width="30" height="24">
<img src="https://getbootstrap.com/docs/{{< param docs_version >}}/assets/brand/bootstrap-logo.svg" alt="Bootstrap" width="30" height="24">
Copy link
Member

Choose a reason for hiding this comment

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

This kind of changes should be reverted. We need the files locally referenced.

@XhmikosR
Copy link
Member

Please split this kind of PRs into smaller ones. You have all kinds of different changes. I also left some comments from a very quick review.

@XhmikosR XhmikosR marked this pull request as draft July 25, 2023 05:21
@louismaximepiton louismaximepiton force-pushed the main-lmp-js-stackblitz branch from 9278923 to 625ca18 Compare July 25, 2023 09:15
@louismaximepiton
Copy link
Member Author

Most of the PR has been split.

@louismaximepiton louismaximepiton deleted the main-lmp-js-stackblitz branch October 9, 2024 08:51
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.

[Tracking] Fix StackBlitz examples
3 participants