Conversation
github-project-automation
Bot
moved this to In Progress / Draft
in 🟣 [Orange-Boosted-Bootstrap] PRs Board
✅ Deploy Preview for boosted ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
# Conflicts: # site/src/assets/partials/snippets.js
# Conflicts: # site/src/assets/partials/snippets.js
MaxLardenois
left a comment
MaxLardenois
left a comment
There was a problem hiding this comment.
LGTM tbh just some stuff
| --- | ||
| title: Tables | ||
| title: Table | ||
| description: Documentation and examples for opt-in styling of tables (given their prevalent use in JavaScript plugins) with OUDS Web. |
There was a problem hiding this comment.
I do not understand the part in parenthesis here
There was a problem hiding this comment.
Ok I understand after reading the Overview, still I think it's not really necessary to have this detail in the subtitle...
|
|
||
| // For rows | ||
| .table-striped { | ||
| .table-zebra { |
There was a problem hiding this comment.
As we do not have design spec I think we can keep striped for now
Co-authored-by: Maxime Lardenois <maxime.lardenois@orange.com>
MaxLardenois
left a comment
MaxLardenois
left a comment
There was a problem hiding this comment.
LGTM with two suggestions
boosted-bot
moved this from In Progress / Draft
to Need Lead Dev Review
in 🟣 [Orange-Boosted-Bootstrap] PRs Board
boosted-bot
moved this from Need Lead Dev Review
to Need Dev Review
in 🟣 [Orange-Boosted-Bootstrap] PRs Board
| $table-striped-color: $table-color !default; | ||
| $table-striped-bg-factor: .035 !default; // OUDS mod: equivalent to `$gray-200` | ||
| $table-striped-bg: rgba(var(--#{$prefix}black-rgb), var(--#{$prefix}table-striped-bg-factor)) !default; // OUDS mod: instead of `rgba(var(--#{$prefix}emphasis-color-rgb), $table-striped-bg-factor)` | ||
| $table-striped-bg: var(--#{$prefix}color-bg-secondary) !default; // OUDS mod: instead of `rgba(var(--#{$prefix}emphasis-color-rgb), $table-striped-bg-factor)` |
There was a problem hiding this comment.
Shouldn't we use the surface secondary here ? it would make it work even if the table is already on a secondary background thank to the opacity.
| $table-cell-padding-y-sm: .5625rem !default; // OUDS mod | ||
| $table-cell-padding-x-sm: $table-cell-padding-x !default; // OUDS mod | ||
| $table-cell-padding-y: $core-ouds-dimension-200 !default; // OUDS mod | ||
| $table-cell-padding-x: $core-ouds-dimension-200 !default; // OUDS mod |
There was a problem hiding this comment.
Should we use a semantic token to have a smaller padding on Orange Compact ?
The $ouds-dimension-5xsmall seems to be 16px in Orange and 12px on Orange Compact. Might be interesting. WDYT ?
| <br/> | ||
| Resize window to see differences: | ||
| {getData('breakpoints').map((breakpoint) => { | ||
| return ( |
There was a problem hiding this comment.
Maybe we could add a small title before each table just to tell which breakpoint is use in the following table ?
| <details class="mt-medium mb-2xlarge"> | ||
| <summary>{summary}</summary> | ||
| <slot /> | ||
| </details> |
There was a problem hiding this comment.
Good idea, I wonder how many details/summary can be replaced by this shortcode. Did you do a search by any chance, do you think it's worth creating an issue to replace them ?
4 participants
Types of change
Related issues
Closes #3463
Context & Motivation
Description
Checklists
Checklist (for Core Team only)
Progression (for Core Team only)
ouds/mainfollowing conventional commitLive previews