[Tidy] Replace custom classNames with BS utility CSS classes

[huong-li-nguyen]

Description

Our long-term objective is to minimize the use of custom CSS in the static folder, favoring Bootstrap syntax instead. To achieve this, we can replace custom class names with Bootstrap utility CSS classes: https://getbootstrap.com/docs/4.0/utilities/borders/.

This PR shows one example on how to do so. However, I don't recommend replacing all custom class names with Bootstrap shortcuts, as it could make it harder for users to override styles.

Here are my current guidelines:

• Use Bootstrap classes instead of custom class names when it's unlikely that users will want to customize the HTML element via CSS, or when there is an alternative method for customization.
• Use custom class names for HTML elements that users are likely to want to customize e.g. all our main containers (page-header, control-panel, right-side, left-side etc.)
• Not related to above: Remove any redundant extra div if you see one

See example here where we could simplify using BS shortcuts: #910 (comment)

Screenshot

Notice

• I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":

  • I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
  • I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorized to submit this contribution on behalf of the original creator(s) or their licensees.
  • I certify that the use of this contribution as authorized by the Apache 2.0 license does not violate the intellectual property rights of anyone else.
  • I have not referenced individuals, products or companies in any commits, directly or indirectly.
  • I have not added data or restricted code in any commits, directly or indirectly.

[github-actions]

View the example dashboards of the current commit live on PyCafe ☕ 🚀

Updated on: 2024-12-03 12:20:40 UTC
Commit: ca2032f

Link: vizro-core/examples/dev/

Link: vizro-core/examples/scratch_dev

Link: vizro-core/examples/visual-vocabulary/

Link: vizro-ai/examples/dashboard_ui/

[nadijagraca]

Looks good to me! 🙌

[antonymilne]

Generally agree with everything you said, although I am probably more pro-bootstrap native than what you suggest. My proposed rule of thumb would be something like: use bootstrap shortcuts as much as possible. But if that's not practical then I agree with what you say 👍

Use Bootstrap classes instead of custom class names when it's unlikely that users will want to customize the HTML element via CSS, or when there is an alternative method for customization.

Agree.

Use custom class names for HTML elements that users are likely to want to customize e.g. all our main containers (page-header, control-panel, right-side, left-side etc.)

These are currently done with id rather than className, so we can potentially combine both our custom CSS and bootstrap and a user can still further customise them?

html.Div(id="page-header", classNames="align-items-left ...")

Then in our CSS we just provide any extra bits that we need. We wouldn't do any bootstrap overrides here, just provide extra attributes:

#page-header {
    color: red; /* Some attribute not specified in bootstrap */
}

And if a user wanted to change something they would do:

#page-header {
   color: red; /* Some attribute not specified in bootstrap */
   align-items: right !important; /* Override something specified in boostrap */
}

The only thing that's awkward here is the !important that's needed, and maybe if that's already used in bootstrap it doesn't even work well in practice? If that's the case then agree with what you say and I'm fine to just do this like:

html.Div(id="page-header")

#page-header {
    align-items: left; /* All the things done by the bootstrap styles put here manually */
    color: red;
}

Not related to above: Remove any redundant extra div if you see one

Always agree with this 👍

[huong-li-nguyen]

The only thing that's awkward here is the !important that's needed, and maybe if that's already used in bootstrap it doesn't even work well in practice? If that's the case then agree with what you say and I'm fine to just do this like:

Yes, that's why I wasn't more supportive of using native Bootstrap everywhere. When you use any of Bootstrap's utility classes, they automatically include the !important tag.

For example, if you write: html.Div(id="page-header", classNames="align-items-left")

Bootstrap will apply the following CSS:

#page-header {
   align-items: left !important;
}

This forces the user to also use the !important tag in their CSS to override it. Normally, user-defined CSS should take precedence since it is loaded last, so there shouldn't be a conflict. I am sure that's also what dcc thought, but in the end we never know how they intend to use it 😄 For instance, we also had difficulty styling some dcc components because they used the !important tags in some of their components. This always led to confusion.

In our case, it would even be more confusing as our users would then need to inspect elements to differentiate between CSS properties from our static folder and Bootstrap's classes with !important tags, and their own styles. So I felt that approach would just lead to them overusing the !important tag which in general is bad practice and would lead to confusion and potential errors 🤔

[huong-li-nguyen]

I've already merged this PR, but we can continue discussing our future strategy. I plan to replace all other custom class names with Bootstrap utilities for those elements that users are unlikely to want to customize.

For elements where users might want to apply custom styles, I'll leave the class names unchanged for now, pending our discussion above. 👍