Skip to content

Conversation

@julien-deramond
Copy link
Contributor

@julien-deramond julien-deramond commented Sep 26, 2023

Maintaining retro compatibility with dark variants would cause too many issues:

  • Risk of maintaining rules like the one dropped here in _root.scss which could impact the rest of the framework
  • CSS bundle size
  • Misunderstanding for the users if they can/must use the dark variants or not
  • Complexity in validation to test light mode/light mode dark variant/dark mode/dark mode dark variant for each component

So this PR:

  • drops the extra code found in _root.scs to handle dark variants.
  • drops the specific code of dark variants in _{component}.scss files.
  • drops the corresponding Sass variables in _variables.scss.
  • specifies a deprecation message in the documentation.
  • removes the examples of the dark variant renderings.
  • Accordion
    • ⚠️ Footer partial and Orange footer will be broken temporarily since they use .accordion-dark
    • Live preview
  • Breadcrumb
  • ⚠️ Dropdown and its .dropdown-menu-dark are kept for now since Bootstrap still has it in its documentation. This is also linked to the topics: are headers supposed to use data-bs-theme="dark" or not. Not in the scope of this PR.
  • Buttons
    • Live preview
    • button-variant has its last parameter removed since used only to handle .btn-inverse
    • ⚠️ Tarteaucitron rendering is broken because used .btn-inverse
    • ⚠️ Dark dropdowns are broken
    • ⚠️ Buttons in navbar are broken
    • ⚠️ Card shortcode layout is broken with dark background
    • ⚠️ Orange Footer shortcode is broken for the social buttons and sign up
  • Close button
    • Live preview
    • Dropped usage of .btn-close-white in the documentation as it is deprecated by Bootstrap but kept it in the code for retro compatibility
    • ⚠️ It breaks dark alerts, that will be deprecated as well
    • ⚠️ Impact on navbars
    • ⚠️ Impact on dark offcanvas that must be kept because of Bootstrap
    • ⚠️ Impact on toast color schemes
  • Dropdown
    • Dark dropdowns are kept for now since Bootstrap still has them in its doc
    • Live preview
  • List group
  • Local navigation
    • ⚠️ The dark variant needed to apply .navbar-dark inside its DOM... Also linked to the headers, but hope it's gonna work without .navbar-dark. Let's see when we'll develop this component
    • Live preview
  • ⚠️ Navbar and its .navbar-dark are kept for now since Bootstrap still has it in its documentation. This is linked to the topic: are headers supposed to use data-bs-theme="dark" or not. Not in the scope of this PR.
  • Offcanvas
    • Live preview
    • Kept the exact same description as in Bootstrap. The example still working because we kept .btn-close-white also still available in Bootstrap
  • Pagination
  • Stepped process
  • Tags

@julien-deramond julien-deramond added the color mode Temporary label to highlight color mode issues label Sep 26, 2023
@netlify
Copy link

netlify bot commented Sep 26, 2023

Deploy Preview for boosted ready!

Name Link
🔨 Latest commit 608cd4c
🔍 Latest deploy log https://app.netlify.com/sites/boosted/deploys/651bd6623603930008218c1f
😎 Deploy Preview https://deploy-preview-2270--boosted.netlify.app/docs/5.3/components/dropdowns
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@julien-deramond julien-deramond changed the base branch from main to main-jd-dark-mode September 26, 2023 12:01
@julien-deramond julien-deramond marked this pull request as ready for review September 27, 2023 08:15
@julien-deramond julien-deramond marked this pull request as draft September 28, 2023 11:25
</div>
</div>
{{< /example >}}
Add `data-bs-theme="dark"` to the `.accordion` or any ancestor element to enable a component-specific color mode. [Learn more about our color modes]({{< docsref "/customize/color-modes" >}}).
Copy link
Member

Choose a reason for hiding this comment

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

Note for later: we will have to check wether users need to add a background in addition to theme, and if so, mention it there. Same for other components

@julien-deramond julien-deramond force-pushed the main-jd-dark-mode-drop-dark-variants branch from bb078d6 to 36cb3f0 Compare October 2, 2023 07:03
@julien-deramond julien-deramond force-pushed the main-jd-dark-mode-drop-dark-variants branch from 36cb3f0 to 70c6f34 Compare October 2, 2023 10:23
Copy link
Member

@louismaximepiton louismaximepiton left a comment

Choose a reason for hiding this comment

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

Here is a first review on what was done, I'll continue to check if nothing was forgotten.

FWIW, for consistency, I'd drop the .dropdown-menu-dark, .navbar-dark and .btn-close-white as well Bootstrap side.


$mark-padding: 0 .1875em !default; // Boosted mod
$mark-color: $white !default; // Boosted mod: instead of `$body-color`
$mark-bg: $black !default; // Boosted mod: instead of `$yellow-100`
Copy link
Member

Choose a reason for hiding this comment

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

[Out of PR] We might not need to precise it's a Boosted mod since we are already inside one (since l.719)

@louismaximepiton
Copy link
Member

louismaximepiton commented Oct 2, 2023

As dark variants, we might remove as well:

Copy link
Member

@louismaximepiton louismaximepiton left a comment

Choose a reason for hiding this comment

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

Two small questions.

@julien-deramond julien-deramond marked this pull request as ready for review October 3, 2023 09:07
Copy link
Member

@louismaximepiton louismaximepiton left a comment

Choose a reason for hiding this comment

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

👌

@julien-deramond julien-deramond merged commit 938cb59 into main-jd-dark-mode Oct 3, 2023
@julien-deramond julien-deramond deleted the main-jd-dark-mode-drop-dark-variants branch October 3, 2023 09:12
@julien-deramond julien-deramond mentioned this pull request Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

color mode Temporary label to highlight color mode issues

Projects

Development

Successfully merging this pull request may close these issues.

3 participants