Repository navigation
Conversation
✅ Deploy Preview for boosted ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…t) + add marker class on homepage
# Conflicts: # site/src/components/home/GetStarted.astro
There was a problem hiding this comment.
🟡 Changes recommended
The implementation omits .fs-hl, regresses Sosh homepage styling, and directly edits generated token files.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a brand-specific marker for heading-large typography.
Changes:
- Implements the marker using tokenized SVG masking.
- Documents and applies it to homepage headings.
- Updates CSS snapshots and package metadata.
File summaries
| File | Description |
|---|---|
site/src/content/docs/foundation/typography.mdx |
Documents marker usage. |
site/src/components/home/GetStarted.astro |
Applies marker and adjusts subheadings. |
site/src/components/home/Customize.astro |
Applies marker and adjusts subheadings. |
site/src/components/home/CSSVariables.astro |
Applies marker and adjusts subheadings. |
scss/_type.scss |
Defines marker pseudo-element styling. |
scss/tests/snapshot-tests/__snapshots__/ouds-web.css |
Updates compiled CSS snapshot. |
scss/tests/snapshot-tests/__snapshots__/ouds-web-bootstrap.css |
Updates Bootstrap CSS snapshot. |
packages/orange/scss/tokens/_composite.scss |
Defines Orange marker asset dimensions. |
packages/orange/scss/tokens/_component.scss |
Adds Orange marker color token. |
packages/orange-compact/scss/tokens/_composite.scss |
Defines compact marker asset dimensions. |
packages/orange-compact/scss/tokens/_component.scss |
Adds compact marker color token. |
packages/sosh/scss/tokens/_composite.scss |
Disables the marker asset for Sosh. |
packages/sosh/scss/tokens/_component.scss |
Adds the null Sosh marker color. |
package.json |
Reorders DocSearch dependencies. |
package-lock.json |
Rewrites dependency metadata. |
Review details
Files not reviewed (2)
- scss/tests/snapshot-tests/snapshots/ouds-web-bootstrap.css: Generated file
- scss/tests/snapshot-tests/snapshots/ouds-web.css: Generated file
- Files reviewed: 12/15 changed files
- Comments generated: 9
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
# Conflicts: # package-lock.json
- use both token $ouds-typography-heading-large-marker and presence of an asset to display marker
…134918' into main-his-typo-headings # Conflicts: # packages/orange-compact/scss/tokens/_component.scss # packages/orange/scss/tokens/_component.scss # packages/sosh/scss/tokens/_component.scss # site/data/_components-versions.yml
* feat(lib): Bump Items to v1.2.0 * fix review Co-authored-by: Vincent Prothais <vincent.prothais@orange.com> * fix review Co-authored-by: Vincent Prothais <vincent.prothais@orange.com> --------- Co-authored-by: Vincent Prothais <vincent.prothais@orange.com>
| # Typography | ||
|
|
||
| Global typography settings, headings, body text, display headings, and text utilities. | ||
| Typography is a set of UI elements that structure and style text content: headings, display text, body text, |
There was a problem hiding this comment.
@louismaximepiton I'm wondering about the strange indentation? Should I fix it?
louismaximepiton
left a comment
There was a problem hiding this comment.
Do you think that we should somehow propose a way to copy our configuration for the Prism syntax coloration ?
Haven't reviewed the skills yet since there is many to change before to review it
| code { | ||
| font-size: $code-font-size; | ||
| font-style: normal; // OUDS mod: <var> is italic in all browsers | ||
| color: var(--#{$prefix}color-content-muted); |
There was a problem hiding this comment.
Still not a big fan of removing it, since all documentation generators will generate default color code which is barely visible imo and will encourage people to use every color they can use, for example $ouds-color-content-status-warning which is imo an error. I'll keep this open to have other thoughts.
For instance if we keep it like that, we should also remove :
- $code-color and its associated CSS variable
- Add something in the update guide
There was a problem hiding this comment.
To confirm with @vprothais wether we keep the grey color or not?
| <ThemeSpecific theme={['orange', 'orange-compact']}> | ||
| ### Heading with marker | ||
|
|
||
| A brand-colored marker can be displayed below the heading large (`<h2>` or `.h2`), to enhance its visual emphasis and reinforce information hierarchy, by adding the `.marker class`. |
There was a problem hiding this comment.
Not sure we want to keep only marker, which is not specific enough to me, maybe something like title-marker or something around this
There was a problem hiding this comment.
It could be "heading-marker" but if after they want to use it on something else than a heading... Or "typography-marker" ut a bit long. @vprothais what do you think?
Co-authored-by: Louis-Maxime Piton <louismaxime.piton@orange.com>
# Conflicts: # site/src/content/docs/getting-started/update-guide.mdx # skills/using-ouds-web-version-major-minor/references/getting-started/update-guide.md
louismaximepiton
left a comment
There was a problem hiding this comment.
LGTM, let's send it to design and a11y review and Vincent may give its thoughts in the meantime
Types of change
Related issues
Closes #3655
Context & Motivation
Description
<pre>/<code>: use content/default color for the component<code>: use the content/muted color four our inline code elements in the doc.fs-hl.blockquotedeleted, use<blockquote>insteas, associated with.blockquote-footerfor authoringSome TODOs are still in the code and still to check with reviewer.
Checklists
Checklist (for Core Team only)
Progression (for Core Team only)
mainfollowing conventional commitLive previews
https://deploy-preview-3776--boosted.netlify.app/orange/docs/foundation/reboot/
https://deploy-preview-3776--boosted.netlify.app/orange-compact/docs/foundation/reboot/
https://deploy-preview-3776--boosted.netlify.app/sosh/docs/foundation/reboot/
https://deploy-preview-3776--boosted.netlify.app/orange/docs/components/typography/
https://deploy-preview-3776--boosted.netlify.app/orange-compact/docs/components/typography/
https://deploy-preview-3776--boosted.netlify.app/sosh/docs/components/typography/
https://deploy-preview-3776--boosted.netlify.app/orange/docs/utilities/text/#font-size
https://deploy-preview-3776--boosted.netlify.app/orange-compact/docs/utilities/text/#font-size
https://deploy-preview-3776--boosted.netlify.app/sosh/docs/utilities/text/#font-size