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

Examples: new app download page #1915

Closed
wants to merge 55 commits into from
Closed

Conversation

louismaximepiton
Copy link
Member

@louismaximepiton louismaximepiton commented Mar 22, 2023

Note: Please transform - [ ] into - (NA) in the description when things are not applicable

Related issues

Closes #1888.

Description

Adding a new example page to show how to make a download page for apps.

Dev questions:

  • Where to put the images related to the example, I messed up a bit with the folder structure in here.
  • Should I introduce .border-translucent to Bootstrap directly ?

Motivation & Context

To help people download our apps.

Types of change

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

Live previews

Checklist

Contribution

Accessibility

  • My change follows accessibility good practices; I have at least run axe

Design

  • My change respects the design guidelines defined in Orange Design System
  • My change is compatible with responsive display

Development

  • My change follows the developer guide
  • (NA) I have added JavaScript unit tests to cover my changes
  • (NA) I have added SCSS unit tests to cover my changes

Documentation

  • My change introduces changes to the documentation and/or I have updated the documentation accordingly

Checklist (for Core Team only)

  • My change introduces changes to the migration guide
  • My new component is added in Storybook
  • My new component is compatible with RTL
  • Manually run BrowserStack tests
  • Manually test browser compatibility with BrowserStack (Chrome >= 60, Firefox >= 60 (+ ESR), Edge, Safari >= 12, iOS Safari, Chrome & Firefox on Android)
  • Code review
  • Design review
  • A11y review

After the merge

@netlify
Copy link

netlify bot commented Mar 22, 2023

Deploy Preview for boosted ready!

Name Link
🔨 Latest commit ae63096
🔍 Latest deploy log https://app.netlify.com/sites/boosted/deploys/64b682963b87e20008305cb9
😎 Deploy Preview https://deploy-preview-1915--boosted.netlify.app/docs/5.3/examples/download-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 configuration.

@B3nz01d
Copy link
Collaborator

B3nz01d commented Apr 19, 2023

@louismaximepiton louismaximepiton marked this pull request as ready for review April 20, 2023 09:55
@MewenLeHo MewenLeHo self-requested a review April 24, 2023 12:43
Copy link
Contributor

@Aniort Aniort left a comment

Choose a reason for hiding this comment

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

All issues corrected,
Good to me !

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.

Got to switch to something else, but here is a first batch of comments.

@julien-deramond
Copy link
Member

@louismaximepiton There's probably something missing (to double-check).
When you run the following, some images don't seem to be available/copied:

$ npm run release
$ unzip boosted-5.3.0-alpha3-examples.zip
$ cd boosted-5.3.0-alpha3-examples
$ php -S localhost:8008 # or something equivalent

@louismaximepiton
Copy link
Member Author

Tried it and the images seem to be recognized and copied. However, it seems that there's an issue with additional CSS and Boosted Js at least.

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.

Haven't reviewed everything yet. Here is my first feedback.

site/content/docs/5.3/examples/download-app/index.html Outdated Show resolved Hide resolved
site/content/docs/5.3/examples/download-app/index.html Outdated Show resolved Hide resolved
site/layouts/partials/stylesheet.html Outdated Show resolved Hide resolved
site/content/docs/5.3/examples/download-app/index.html Outdated Show resolved Hide resolved
site/content/docs/5.3/examples/download-app/index.html Outdated Show resolved Hide resolved
site/content/docs/5.3/examples/download-app/index.html Outdated Show resolved Hide resolved
site/content/docs/5.3/examples/download-app/index.html Outdated Show resolved Hide resolved
site/content/docs/5.3/examples/download-app/index.html Outdated Show resolved Hide resolved
site/content/docs/5.3/examples/download-app/index.html Outdated Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Jul 18, 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 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

const toggle = document
.createRange()
.createContextualFragment(
`<div class="form-check form-switch my-2 my-lg-0"><input class="form-check-input ms-0" type="checkbox" id="mastermediaAllowed" aria-describedby="tacCLmastermedia" onchange="${choiceEvent}"${((document.cookie.match(/^(?:.*;)?\s*cookie-consent\s*=\s*([^;]+)(?:.*)?$/) || [null])[1].match('!mastermedia=true') ? 'checked' : '')}><label class="form-check-label visually-hidden" for="mastermediaAllowed">Google Tag Manager</label><input id="mastermediaDenied" class="d-none"></div>`
Copy link
Member

Choose a reason for hiding this comment

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

"Google Tag Manager" should prolly be changed

{{- if eq .Page.Title "Download app" -}}
{{- $vendor := resources.Match "js/vendor/*.js" -}}
{{- $js := resources.Match "js/*.js" -}}
{{- $targetDocsJSPath := path.Join "/docs" .Site.Params.docs_version "assets/js/docs.js" -}}
Copy link
Member

Choose a reason for hiding this comment

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

We should probably split docs.js as we did for docs.css

@julien-deramond
Copy link
Member

All comments have been taken into account. Apparently, there are no more Mastermedia cookies, or we dreamt there were Mastermedia cookies at some point. So we close this PR which is now superseded by #2146

@louismaximepiton louismaximepiton deleted the main-lmp-download-example branch July 20, 2023 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Page Example > Application Multi-Download page
6 participants