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

Add new info callout for ODS recommendation #1709

Merged
merged 14 commits into from
Mar 13, 2023

Conversation

hannahiss
Copy link
Member

@hannahiss hannahiss commented Dec 15, 2022

Related issues

Linked to issue #1357 and PR #1674

Description

Add a new blue callout of level "info" to inform user that recommended tables are the small ones, with class .table-sm (and not the "default" one).
This PR stays in draft as long as the PR #1614 about design callouts is not merged. After that, and if we still want to add recommendation callout, we'll have to think about implementing all kinds of alerts (success and warning too) in a single piece of code.

Motivation & Context

This new callout has been discussed in web weekly spec review. Standard tables in DSM correspond to small tables in Boosted, and it wasn't mentioned anywhere. This callout could be used to make it clear to user, in addition to a renaming of tables in DSM.

Types of change

  • New feature (non-breaking change which adds functionality)

Live previews

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
  • My change introduces changes to the migration guide
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

@sonarcloud
Copy link

sonarcloud bot commented Dec 15, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@netlify
Copy link

netlify bot commented Dec 15, 2022

Deploy Preview for boosted ready!

Name Link
🔨 Latest commit 9790d14
🔍 Latest deploy log https://app.netlify.com/sites/boosted/deploys/640ef09a1c3735000847dc76
😎 Deploy Preview https://deploy-preview-1709--boosted.netlify.app
📱 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 settings.

@hannahiss hannahiss changed the title Add new info callout for ODs recommendation Add new info callout for ODS recommendation Dec 15, 2022
@julien-deramond julien-deramond added v5 docs Improvements or additions to documentation labels Dec 16, 2022
@julien-deramond
Copy link
Member

I suppose this will be merged after the callouts PR. Just a comment for future selves: this PR will need an update of https://deploy-preview-1614--boosted.netlify.app/docs/5.2/getting-started/introduction/#about-orange-brand because it won't be anymore only "red"/"danger" messages/callouts.

# Conflicts:
#	site/content/docs/5.2/content/tables.md
@hannahiss hannahiss marked this pull request as ready for review January 16, 2023 08:48
site/assets/scss/_alerts.scss Outdated Show resolved Hide resolved
site/layouts/shortcodes/ods-recommendation-alert.html Outdated Show resolved Hide resolved
site/layouts/shortcodes/ods-recommendation-alert.html Outdated Show resolved Hide resolved
site/layouts/shortcodes/ods-recommendation-alert.html Outdated Show resolved Hide resolved
site/layouts/shortcodes/ods-recommendation-alert.html Outdated Show resolved Hide resolved
site/layouts/shortcodes/ods-recommendation-alert.html Outdated Show resolved Hide resolved
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.

Only 2 things more to change before approving!

site/content/docs/5.2/content/tables.md Outdated Show resolved Hide resolved
site/layouts/shortcodes/ods-alert.html Outdated Show resolved Hide resolved
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.

LGTM 🚀

Copy link
Member

@julien-deramond julien-deramond left a comment

Choose a reason for hiding this comment

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

As a general comment, IMO the About Orange Brand section should also mention this new way of displaying recommendations. What do you think?
(see #1709 (comment))

site/layouts/shortcodes/ods-alert.html Outdated Show resolved Hide resolved
site/layouts/shortcodes/ods-alert.html Outdated Show resolved Hide resolved
site/layouts/shortcodes/ods-alert.html Outdated Show resolved Hide resolved
site/content/docs/5.3/content/tables.md Outdated Show resolved Hide resolved
site/content/docs/5.3/content/tables.md Outdated Show resolved Hide resolved
site/content/docs/5.3/content/tables.md Outdated Show resolved Hide resolved
hannahiss and others added 4 commits March 6, 2023 16:39
Co-authored-by: Julien Déramond <julien.deramond@orange.com>
Co-authored-by: Julien Déramond <julien.deramond@orange.com>
Co-authored-by: Julien Déramond <julien.deramond@orange.com>
- removing unneeded boosted mod comments
- adding info about blue design callout in introduction and update text about red ones
- removing h4 class to callout title

Also:
- renaming ods-alert shortcode to design-callout-alert to be easier to find and refer
@sonarcloud
Copy link

sonarcloud bot commented Mar 13, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Member

@julien-deramond julien-deramond left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@julien-deramond julien-deramond merged commit ae6cfe9 into main Mar 13, 2023
@julien-deramond julien-deramond deleted the main-his-tables-reco-callouts branch March 13, 2023 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to documentation v5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants