gooroodev commented on PR #19717:
URL: https://github.com/apache/echarts/pull/19717#issuecomment-2131318981
### Summary of Changes
1. **ESLint Configuration**:
- Updated `tsconfig.json` path to include all subdirectories.
- Added `browser` environment.
2. **Tooltip Component**:
- Refactored `assembleArrow` function to use CSS classes instead of
inline styles.
- Introduced helper functions `calculateArrowOffset` and
`getColorClassName`.
- Refactored `setContent` method to use helper functions `clearContent`,
`setTextContent`, and `appendArrow`.
3. **Tooltip Markup**:
- Removed inline styles and replaced them with CSS classes.
- Refactored `getTooltipTextStyle` to return CSS class names instead of
inline styles.
- Refactored `wrapBlockHTML`, `wrapInlineNameHTML`, and
`wrapInlineValueHTML` to use CSS classes.
4. **Format Utility**:
- Updated `getTooltipMarker` to use CSS classes instead of inline styles.
5. **Test HTML Files**:
- Added `basic.css` file to include necessary styles.
- Added `Content-Security-Policy` meta tag to enforce strict CSP.
- Removed inline styles and replaced them with styles from `basic.css`.
6. **CSS File**:
- Created `basic.css` with necessary styles for tooltips, markers, and
layout.
### Issues, Bugs, and Typos
1. **Code Duplication**:
- `calculateArrowOffset` and `getColorClassName` functions are defined
inside `assembleArrow`. They can be moved outside for better readability and
reusability.
2. **Inconsistent CSS Class Naming**:
- The class names for colors and other properties are inconsistent.
Consider using a consistent naming convention for better maintainability.
3. **Unused Imports**:
- Ensure no unused imports or variables are present in the updated files.
### Proposed Code Improvements
1. **Move Helper Functions Outside**:
```typescript
function calculateArrowOffset(rotatedWH: number, borderWidth: number,
arrowWH: number) {
return Math.round(
(
((rotatedWH - Math.SQRT2 * borderWidth) / 2
+ Math.SQRT2 * borderWidth
- (rotatedWH - arrowWH) / 2)
* 100
) / 100
);
}
function getColorClassName(color: ZRColor) {
const colorValue = convertToColorString(color);
return colorValue.replace(/[^a-zA-Z0-9]/g, '');
}
```
2. **Consistent CSS Class Naming**:
- Ensure that the class names follow a consistent pattern, e.g.,
`tooltip-arrow-color-fff` instead of `tooltip-arrow-background-color-fff`.
### General Review of Code Quality and Style
1. **Code Quality**:
- The refactoring improves the separation of concerns by moving styles
to CSS classes.
- The use of helper functions enhances code readability and
maintainability.
- The use of CSS classes instead of inline styles makes the code more
compliant with CSP.
2. **Code Style**:
- The code follows good practices such as modularization and reusability.
- The naming conventions for variables and functions are clear and
descriptive.
- The indentation and formatting are consistent and adhere to standard
coding guidelines.
3. **Testing**:
- Ensure that all changes are covered by existing or new tests.
- Verify that the changes do not introduce any regressions or new issues.
Overall, the pull request significantly improves the compliance with strict
CSP by removing inline styles and using CSS classes. The proposed changes
enhance the maintainability and readability of the code.
Yours, [Gooroo.dev](https://gooroo.dev). To receive reviews automatically,
[install Github App](https://github.com/apps/gooroo-dev)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
