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 initial docs for the analysis-report package #21878

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

schlessera
Copy link
Contributor

Context

We are lacking documentation on how to use some of the packages that are meant to be shared across platforms/projects. One of these packages is the analysis-report package.

This PR creates an initial README.md file for that package.

Summary

This PR can be summarized in the following changelog entry:

  • Adds a packages/analysis-report/README.md file with a documentation overview

Relevant technical choices:

  • This was mostly generated via AI and needs review.

@coveralls
Copy link

coveralls commented Nov 29, 2024

Pull Request Test Coverage Report for Build e1d7db12c0eb683296d24082f255725f3c30154c

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+19.5%) to 54.693%

Totals Coverage Status
Change from base Build af60b4214ac8935de070f3e0c222525f92381fd5: 19.5%
Covered Lines: 29753
Relevant Lines: 54692

💛 - Coveralls

Copy link
Contributor

@mhkuu mhkuu left a comment

Choose a reason for hiding this comment

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

I'm not super familiar with this package but I added some initial comments. Again, haven't checked the code snippets. We can consider also referencing our own implementation in WordPress and possibly also the implementation in TYPO3 as usage examples?


## Overview

This package provides React components to display the results of content analysis performed by the [@yoast/yoastseo](../yoastseo) package. It visualizes SEO scores, readability metrics, and improvement suggestions in an organized and user-friendly way.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This package provides React components to display the results of content analysis performed by the [@yoast/yoastseo](../yoastseo) package. It visualizes SEO scores, readability metrics, and improvement suggestions in an organized and user-friendly way.
This package provides React components to display the results of content analysis performed by the `yoastseo` package ([GitHub](../yoastseo), [NPM](https://www.npmjs.com/package/yoastseo)). It visualizes SEO scores, readability metrics, and improvement suggestions in an organized and user-friendly way.

Comment on lines +15 to +19
Note: This package has peer dependencies that need to be installed:
- react
- react-dom
- styled-components
- @wordpress/i18n
Copy link
Contributor

Choose a reason for hiding this comment

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

This bit can be removed in my opinion, they're not peer dependencies but actual dependencies.

```

#### Features
- Sanitizes HTML in text content
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Sanitizes HTML in text content
- Sanitizes HTML in the analysis feedback

Comment on lines +228 to +229
C --> E[MarkerButtons]
C --> F[EditButton]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
C --> E[MarkerButtons]
C --> F[EditButton]
C --> E[Highlighting button]
C --> F[Edit button (Premium)]
C --> G[Yoast AI Optimize button (Premium)]

When using this package outside of WordPress, keep in mind:

1. **Translations**: The package uses @wordpress/i18n for translations. You'll need to:
- Provide your own translation implementation
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Provide your own translation implementation
- Provide your own translation implementation when used outside of WordPress. The translations can be retrieved from https://translate.wordpress.org.

- Mock the i18n functions in tests
- Set up the translation infrastructure

2. **Styling**: The package uses styled-components and expects certain theme variables:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
2. **Styling**: The package uses styled-components and expects certain theme variables:
2. **Styling**: The package uses `styled-components` and expects certain theme variables:

Comment on lines +418 to +573
hasMarks: boolean; // Whether this result supports text marking
marker?: Marker; // Marking instructions
marksButtonStatus?: "enabled" | "disabled" | "hidden";

// Edit functionality
hasEditButton: boolean; // Whether to show edit button
editFieldName?: string; // Field to focus when edit clicked

// Styling
bulletColor: string; // Color for score indicator
marksButtonClassName?: string;
editButtonClassName?: string;

// Accessibility
ariaLabelMarks: string;
ariaLabelEdit?: string;

// State
pressed: boolean; // Whether marks are active
suppressedText?: boolean;// Grey out the text
}
```

#### Features
- Sanitizes HTML in text content
- Manages button states and interactions
- Handles accessibility attributes
- Supports custom styling

### SiteSEOReport

Displays aggregate SEO scores with a stacked progress bar visualization.

#### Props
```typescript
interface SiteSEOReportProps {
className?: string;
seoAssessmentText: string;
seoAssessmentItems: Array<{
html: string; // HTML content for item description
value: number; // Percentage value (0-100)
color: string; // Color for progress bar segment
}>;
barHeight?: string; // Custom height for progress bar
}
```

#### Features
- Renders stacked progress bars
- Displays score breakdowns
- Supports custom styling and dimensions
- Handles responsive layouts

## Component Interactions

### Data Flow
```mermaid
sequenceDiagram
participant P as Parent App
participant C as ContentAnalysis
participant A as AnalysisResult
participant M as Marker System

P->>C: Analysis Results
C->>A: Individual Result
A->>M: Marking Request
M->>P: Mark Positions
P->>M: Apply Marks
M->>A: Update UI
```

### State Management

The components follow a unidirectional data flow:

1. **Parent Level**
```javascript
{
activeMarker: string | null; // Currently active marker
marks: Mark[]; // Active text marks
results: AnalysisResult[]; // Analysis results
}
```

2. **ContentAnalysis Level**
```javascript
{
collapsibleStates: { // Section collapse states
[category: string]: boolean
}
}
```

3. **AnalysisResult Level**
```javascript
{
isHovered: boolean; // UI hover state
isPressed: boolean; // Marker active state
}
```
Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove these parts as they mostly repeat information from above.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd definitely keep the sequence diagram, that's nice to have. This last State Management section I don't really understand, but seems different from what I read before as well.

### ContentAnalysis
The main component that organizes analysis results into collapsible sections:

#### Props
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a couple of props missing here. Maybe they're left out intentionally?

markButtonFactory, resultCategoryLabels, onResultChange, renderHighlightUpsell, renderAIFixesButton

### AnalysisResult
Individual result item with score indicator and optional marking functionality:

#### Props
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a couple of props missing here too.

hasMarksButton, hasEditButton, hasAIButton, hasAIFixes, buttonIdMarks, buttonIdEdit, onButtonClickMarks, onButtonClickEdit, marksButtonFactory, hasBetaBadgeLabel, isPremium, onResultChange, shouldUpsellHighlighting, renderHighlightUpsell, renderAIFixesButton

seoAssessmentText: string;
seoAssessmentItems: Array<{
html: string; // HTML content for item description
value: number; // Percentage value (0-100)
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be any numeric value, shouldn't be a percentage value. The SiteSEOReport uses the StackedProgressBar from @yoast/components which will sum all the value props of these items and determines a percentage afterwards. The values should be positives, negative values will be considered to be 0. Note these values are also used for the score breakdown.

image

## Example Integration

```jsx
import { ContentAnalysis, AnalysisResult } from "@yoast/analysis-report";
Copy link
Contributor

Choose a reason for hiding this comment

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

The AnalysisResult isn't used in this example.

Suggested change
import { ContentAnalysis, AnalysisResult } from "@yoast/analysis-report";
import { ContentAnalysis } from "@yoast/analysis-report";

}
```

## Components Reference
Copy link
Contributor

Choose a reason for hiding this comment

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

This section is pretty much a duplicate of the Core Components section, except from the component descriptions that are slightly more detailed here.

Comment on lines +418 to +573
hasMarks: boolean; // Whether this result supports text marking
marker?: Marker; // Marking instructions
marksButtonStatus?: "enabled" | "disabled" | "hidden";

// Edit functionality
hasEditButton: boolean; // Whether to show edit button
editFieldName?: string; // Field to focus when edit clicked

// Styling
bulletColor: string; // Color for score indicator
marksButtonClassName?: string;
editButtonClassName?: string;

// Accessibility
ariaLabelMarks: string;
ariaLabelEdit?: string;

// State
pressed: boolean; // Whether marks are active
suppressedText?: boolean;// Grey out the text
}
```

#### Features
- Sanitizes HTML in text content
- Manages button states and interactions
- Handles accessibility attributes
- Supports custom styling

### SiteSEOReport

Displays aggregate SEO scores with a stacked progress bar visualization.

#### Props
```typescript
interface SiteSEOReportProps {
className?: string;
seoAssessmentText: string;
seoAssessmentItems: Array<{
html: string; // HTML content for item description
value: number; // Percentage value (0-100)
color: string; // Color for progress bar segment
}>;
barHeight?: string; // Custom height for progress bar
}
```

#### Features
- Renders stacked progress bars
- Displays score breakdowns
- Supports custom styling and dimensions
- Handles responsive layouts

## Component Interactions

### Data Flow
```mermaid
sequenceDiagram
participant P as Parent App
participant C as ContentAnalysis
participant A as AnalysisResult
participant M as Marker System

P->>C: Analysis Results
C->>A: Individual Result
A->>M: Marking Request
M->>P: Mark Positions
P->>M: Apply Marks
M->>A: Update UI
```

### State Management

The components follow a unidirectional data flow:

1. **Parent Level**
```javascript
{
activeMarker: string | null; // Currently active marker
marks: Mark[]; // Active text marks
results: AnalysisResult[]; // Analysis results
}
```

2. **ContentAnalysis Level**
```javascript
{
collapsibleStates: { // Section collapse states
[category: string]: boolean
}
}
```

3. **AnalysisResult Level**
```javascript
{
isHovered: boolean; // UI hover state
isPressed: boolean; // Marker active state
}
```
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd definitely keep the sequence diagram, that's nice to have. This last State Management section I don't really understand, but seems different from what I read before as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants