diff --git a/.github/workflows/pr_checklists.yml b/.github/workflows/pr_checklists.yml new file mode 100644 index 00000000000..8fd7362fb14 --- /dev/null +++ b/.github/workflows/pr_checklists.yml @@ -0,0 +1,208 @@ +name: Pull Request Checklists + +on: + pull_request: + types: [opened, synchronize, reopened] + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + checklist: + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Get changed files + id: files + run: | + set -o pipefail + git fetch origin ${{ github.event.pull_request.base.ref }}:${{ github.event.pull_request.base.ref }} + git fetch origin ${{ github.event.pull_request.head.ref }}:${{ github.event.pull_request.head.ref }} + FILES=$(git diff --name-only origin/${{ github.event.pull_request.base.ref }}...HEAD) + if [[ -z "$FILES" ]]; then + echo "No files changed in this PR." > files.txt + else + echo "$FILES" > files.txt + fi + + - name: Determine which checklists to include + id: checklists + run: | + INCLUDE_CSS=false + INCLUDE_ANALYTICS=false + INCLUDE_L10N=false + INCLUDE_EXPERIMENT=false + INCLUDE_HTML=false + INCLUDE_JS=false + + # Loop directly over status and filename from git diff --name-status + while IFS=$'\t' read -r STATUS FILE; do + # Skip if file doesn't exist or is not readable + if [[ ! -f "$FILE" || ! -r "$FILE" ]]; then + echo "Skipping non-existing or unreadable file: $FILE" + continue + fi + + # Only run filename checks for newly added files + if [[ "$STATUS" == "A" ]]; then + [[ "$INCLUDE_CSS" != "true" && "$FILE" == *.scss ]] && INCLUDE_CSS=true + [[ "$INCLUDE_L10N" != "true" && "$FILE" == *.ftl ]] && INCLUDE_L10N=true + [[ "$INCLUDE_HTML" != "true" && "$FILE" == *.html ]] && INCLUDE_HTML=true + [[ "$INCLUDE_JS" != "true" && "$FILE" == *.js ]] && INCLUDE_JS=true + [[ "$INCLUDE_EXPERIMENT" != "true" && "$FILE" == *experiment* ]] && INCLUDE_EXPERIMENT=true + fi + + # Get only the changed lines for this file + CHANGED_LINES=$(git diff origin/${{ github.event.pull_request.base.ref }}...HEAD -- "$FILE" | grep '^+' | grep -v '^+++' | cut -c2-) + + # Content-based checks (apply to all changed files) + if [[ "$INCLUDE_ANALYTICS" != "true" && "$FILE" == *.html ]]; then + if echo "$CHANGED_LINES" | grep -qE '> $GITHUB_OUTPUT + echo "analytics=$INCLUDE_ANALYTICS" >> $GITHUB_OUTPUT + echo "l10n=$INCLUDE_L10N" >> $GITHUB_OUTPUT + echo "experiment=$INCLUDE_EXPERIMENT" >> $GITHUB_OUTPUT + echo "html=$INCLUDE_HTML" >> $GITHUB_OUTPUT + echo "js=$INCLUDE_JS" >> $GITHUB_OUTPUT + + - name: Build comment body + id: comment + run: | + # Ensure the checklist markdown files exist before proceeding. + if [[ ! -d "docs/docs/checklists" ]]; then + echo "Error: docs/docs/checklists directory not found!" >&2 + exit 1 + fi + + # Initial message for the PR comment. + BODY="Thank you for your pull request. Based on the contents you may need to double-check these things:\n" + + # Tracks whether any checklist was appended to the comment. + CHECKLIST_ADDED=false + + # Appends a checklist section to the comment file. + # Usage: append_checklist "Title" "path/to/checklist.md" + append_checklist() { + local title="$1" + local file="$2" + echo -e "\n---\n\n
\n$title\n\n$(cat $file)\n
\n" >> comment.txt + CHECKLIST_ADDED=true + } + + # Start the comment file with the initial message. + echo -e "$BODY" > comment.txt + + # Append each checklist if its flag is set. + [[ "${{ steps.checklists.outputs.css }}" == "true" ]] && append_checklist "๐Ÿงพ CSS Checklist" "docs/docs/checklists/css.md" + [[ "${{ steps.checklists.outputs.analytics }}" == "true" ]] && append_checklist "๐Ÿ“Š Analytics Checklist" "docs/docs/checklists/analytics.md" + [[ "${{ steps.checklists.outputs.l10n }}" == "true" ]] && append_checklist "๐ŸŒ L10n Checklist" "docs/docs/checklists/l10n.md" + [[ "${{ steps.checklists.outputs.experiment }}" == "true" ]] && append_checklist "๐Ÿงช Experiment Checklist" "docs/docs/checklists/experiment.md" + [[ "${{ steps.checklists.outputs.html }}" == "true" ]] && append_checklist "๐Ÿงฑ HTML Checklist" "docs/docs/checklists/html.md" + [[ "${{ steps.checklists.outputs.js }}" == "true" ]] && append_checklist "๐Ÿ“œ JavaScript Checklist" "docs/docs/checklists/js.md" + + # If no checklists were added, do not post a comment, but log a message for workflow visibility. + if [[ "$CHECKLIST_ADDED" == "false" ]]; then + echo "No specific checklists needed for this PR." + echo "should_comment=false" >> $GITHUB_OUTPUT + else + # Add a suggestion link for improving the script to the comment. + echo -e "\nIf you want to suggest an improvement to this script please comment in mozmeao/springfield#396" >> comment.txt + echo "should_comment=true" >> $GITHUB_OUTPUT + # Encode the comment for safe output to the next workflow step. + echo "body=$(cat comment.txt | jq -Rs .)" >> $GITHUB_OUTPUT + fi + + - name: Check for existing comment + id: find-comment + uses: actions/github-script@v7 + with: + script: | + // Find an existing bot comment with the checklist intro text. + const { data: comments } = await github.rest.issues.listComments({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number + }); + + const botComment = comments.find(comment => { + return comment.user.login === 'github-actions[bot]' && + comment.body.includes('Based on the contents you may need to double-check these things'); + }); + + // Output the comment ID and existence flag for later steps. + if (botComment) { + core.setOutput('comment-id', botComment.id); + core.setOutput('exists', 'true'); + } else { + core.setOutput('exists', 'false'); + } + + - name: Update existing comment + if: steps.find-comment.outputs.exists == 'true' && steps.comment.outputs.should_comment == 'true' + uses: actions/github-script@v7 + with: + script: | + // Update the existing bot comment with the new checklist content. + github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: ${{ steps.find-comment.outputs.comment-id }}, + body: JSON.parse(process.env.BODY) + }) + env: + BODY: ${{ steps.comment.outputs.body }} + + - name: Create new comment + if: steps.find-comment.outputs.exists == 'false' && steps.comment.outputs.should_comment == 'true' + uses: actions/github-script@v7 + with: + script: | + // Create a new bot comment with the checklist content. + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + body: JSON.parse(process.env.BODY) + }) + env: + BODY: ${{ steps.comment.outputs.body }} diff --git a/docs/docs/attribution/0001-analytics.md b/docs/docs/attribution/0001-analytics.md index 5737c73d918..67120b7078a 100644 --- a/docs/docs/attribution/0001-analytics.md +++ b/docs/docs/attribution/0001-analytics.md @@ -65,18 +65,11 @@ CTA ("Call to Action") click is intented to track the one or two main actions th The attribute `data-cta-text` must be present to trigger the event. All links to accounts.mozilla.com must also use `data-cta-type`. -| Data Attribute | Expected Value | -| ------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `data-cta-text` * | Text or name of the link (e.g. `Sign Up`, `Start Here`, `Get Relay`, `See your report`, `Read the Manifesto`). | -| | | -| | - This does not need to exactly match the text displayed to the user | -| | - Defining this is useful to group the link clicks across locales | -| | - - this attribute is required | -| `data-cta-position` | Location of CTA on the page (e.g. `primary`, `secondary`, `banner`, `pricing`) | -| `data-cta-type` | fxa-servicename (e.g. `fxa-sync`, `fxa-monitor`, `fxa-vpn`, `monitor`, `relay`) | -| | | -| | - This is to group CTAs by their destination | -| | - Do not use this to identify the element (ie. link, button) | +| Data Attribute | Expected Value | +| -------------- | -------------- | +| `data-cta-text` * | Text or name of the link (e.g. `Sign Up`, `Start Here`, `Get Relay`, `See your report`, `Read the Manifesto`).
- This does not need to exactly match the text displayed to the user
- Defining this is useful to group the link clicks across locales
* This attribute is required | +| `data-cta-position` | Location of CTA on the page (e.g. `primary`, `secondary`, `banner`, `pricing`) | +| `data-cta-type` | fxa-servicename (e.g. `fxa-sync`, `fxa-monitor`, `fxa-vpn`, `monitor`, `relay`)
- This is to group CTAs by their destination
- Do not use this to identify the element (ie. link, button) | | `data-cta-name` | A identifier for this cta that is unique across the entire site. (e.g. `fx20-primarycta`, `wnp118-sfaq-so-special-features`). This is to help with reporting since it is difficult to filter on more than one parameter at a time. | ``` html @@ -93,10 +86,10 @@ Link click is intended to track links that are of interest but not the focus of The attribute `data-link-text` must be present to trigger the event. -| Data Attribute | Expected Value | -| -------------------- | ---------------------------------------------------------------------------------------------------------------------------- | +| Data Attribute | Expected Value | +| -------------- | -------------- | | `data-link-text` * | Text or name of the link (e.g. `Monitor`, `Features`, `Instagram (mozilla)`, `Mozilla VPN`). - * this attribute is required | -| `data-link-position` | Location of CTA on the page (e.g. `topnav`, `subnav`, `body`, `features`) | +| `data-link-position` | Location of CTA on the page (e.g. `topnav`, `subnav`, `body`, `features`) | ``` html

This is text with a simpleexample.

@@ -292,46 +285,40 @@ TrackProductDownload.sendEvent(customEventObject); We are using the custom event `widget_action` to track the behaviour of javascript widgets. -**How do you chose between \`\`widget_action\`\` and \`\`cta_click\`\`?** - -| widget_action | cta_click | -| ----------------------------------------------------------- | -------------------------------------------------------- | -| The action is specific or unique. | The action is \"click\". | -| | | -| *(Only the language switcher changes* *the page language.)* | | -| The user does not leave the page. | It sends the user somewhere else. | -| It requires Javascript to work. | No JS required. | -| It can perform several actions. | It does one action. | -| | | -| *(A modal can be opened and closed.)* | | -| There could be several on the page doing different things. | There could be several on the page doing the same thing. | -| | | -| *(An accordion list of FAQs)* | *(A download button in the header and footer.)* | +**How do you chose between `widget_action` and `cta_click`?** + +| widget_action | cta_click | +| ------------- | --------- | +| The action is specific or unique.
*(example: only the language switcher changes the page language.)* | The action is "click". | +| The user does not leave the page. | It sends the user somewhere else. | +| It requires Javascript to work. | No JS required. | +| It can perform several actions.
*(example: modal can be opened and closed.)* | It does one action.| +| There could be several on the page doing different things.
*(An accordion list of FAQs)* | There could be several on the page doing the same thing.
*(A download button in the header and footer.)*| Properties for use with ``widget_action`` (not all widgets will use all options): - type - **Required.** - The type of widget. - - Examples: \"modal\", \"protection report\", \"affiliate notification\", \"help icon\". - - *Avoid "button" or "link". If you want to track a link or button use \`cta_click\`.* + - Examples: "modal", "protection report", "affiliate notification", "help icon". + - *Avoid "button" or "link". If you want to track a link or button use `cta_click`.* - action - **Required.** - The thing that happened. - - Examples: \"open\", \"accept\", \"timeout\", \"vote up\". + - Examples: "open", "accept", "timeout", "vote up". - *Avoid "click". If you want to track a click use \`cta_click\`.* - text - How is this action labeled to the user? - - Examples: \"Okay\", \"Check your protection report\", \"Get the app\" + - Examples: "Okay", "Check your protection report", "Get the app" - name - Give the widget a name. - - You probably only need this optional attribute if the ``text`` value is not enough to tell the widgets apart. + - You probably only need this optional attribute if the `text` value is not enough to tell the widgets apart. - This can help you group actions from the same widget, or make it easier to find the widget in the reports. - The dashes are not required but they\'re allowed if you want to match the element class or ID. - - Examples: \"dad-joke-banner\", \"focus-qr-code\", \"Join Firefox Modal\" + - Examples: "dad-joke-banner", "focus-qr-code", "Join Firefox Modal" - non_interaction (boolean) - True if the action was triggered by something other than a user gesture. diff --git a/docs/docs/checklists/analytics.md b/docs/docs/checklists/analytics.md new file mode 100644 index 00000000000..815cf614413 --- /dev/null +++ b/docs/docs/checklists/analytics.md @@ -0,0 +1,39 @@ +## Links `` + +- [ ] Has a `data-cta-text` OR `data-link-text` + - [ ] If it has class `mzp-c-button` it is a CTA + - [ ] If it has class `mzp-c-cta-link` it is a CTA + - [ ] If there are two of the same `data-cta-text` include `data-cta-position` + - [ ] Does not have both `data-cta-text` and `data-link-text` +- [ ] If linking to another Mozilla property include `utm` params + - [ ] `utm_source` is `'www.firefox.com` or `www.mozilla.org` + - [ ] `utm_medium` is `referral` + - [ ] if the query string is in a variable it is called `params` and the string includes the `?` +- Download button: + - [ ] Use the appropriate helper, don't hard code these. (`download_firefox_thanks`, `google_play_button`, `apple_app_store_button`) + - [ ] include a download_location if there are multiple buttons on the page + +# Buttons `