refactor!: remove the ALIGN and RENDERING_HINT enums #798
Conversation
BREAKING CHANGES: - The `constants.ALIGN` enum has been removed, use `AlignValue` and `VAlignValue` types instead - The `constants.RENDERING_HINT` enum has been removed without replacement because it wasn't used
WalkthroughThis change set removes the Changes
Sequence Diagram(s)sequenceDiagram
participant Editor
participant Graph
participant Cell
Note over Editor,Graph: Alignment actions (before)
Editor->>Graph: alignCells(ALIGN.LEFT)
Graph->>Cell: setCellStyles('align', ALIGN.LEFT)
Note over Editor,Graph: Alignment actions (after)
Editor->>Graph: alignCells('left')
Graph->>Cell: setCellStyles('align', 'left')
Note over Editor,Graph: Only argument values change from enum to string literal
sequenceDiagram
participant SvgCanvas2D
participant DOM
Note over SvgCanvas2D: Text rendering (before)
SvgCanvas2D->>DOM: createCss(..., align=ALIGN.CENTER, valign=ALIGN.MIDDLE, ...)
Note over SvgCanvas2D: Text rendering (after)
SvgCanvas2D->>DOM: createCss(..., align='center', valign='middle', ...)
Note over SvgCanvas2D: Only parameter values change from enum to string literal
sequenceDiagram
participant CellEditorHandler
participant User
Note over CellEditorHandler: Setting alignment (before)
User->>CellEditorHandler: setAlign(ALIGN.RIGHT)
Note over CellEditorHandler: Setting alignment (after)
User->>CellEditorHandler: setAlign('right')
Note over CellEditorHandler: Only argument values and type signatures change
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/core/src/view/cell/CellRenderer.ts (1)
1061-1064: Code improvement: Stored state.text in a local variableStored
state.textin a local variabletextShapeto avoid repeated property access, which slightly improves code readability and potentially performance when this method is called frequently.packages/core/src/view/plugins/CellEditorHandler.ts (2)
281-282: Refine JSDoc forinitmethod
The comment block was cleaned up; consider expanding it to document resize behavior and listener installation for future maintainability.
699-699: Avoid shadowing class property with localalign
The localconst aligninsidestartEditingshadowsthis.align. Consider renaming tohAlign(or similar) to prevent confusion.packages/core/src/view/canvas/SvgCanvas2D.ts (2)
230-230: Update comment forlineHeightCorrection
The JSDoc now referencesLINE_HEIGHT; adding a brief example or note on its effect could help future readers.
1583-1593: Handlevaligncases for vertical text position
The new logic for'middle'and'bottom'aligns plain text correctly; consider extracting this into a small helper to reduce duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
CHANGELOG.md(1 hunks)packages/core/src/editor/Editor.ts(2 hunks)packages/core/src/serialization/ObjectCodec.ts(1 hunks)packages/core/src/util/Constants.ts(0 hunks)packages/core/src/util/styleUtils.ts(2 hunks)packages/core/src/view/GraphView.ts(4 hunks)packages/core/src/view/canvas/SvgCanvas2D.ts(11 hunks)packages/core/src/view/cell/CellRenderer.ts(3 hunks)packages/core/src/view/cell/CellState.ts(2 hunks)packages/core/src/view/geometry/node/LabelShape.ts(4 hunks)packages/core/src/view/geometry/node/TextShape.ts(6 hunks)packages/core/src/view/geometry/stencil/StencilShape.ts(1 hunks)packages/core/src/view/mixins/CellsMixin.ts(5 hunks)packages/core/src/view/mixins/CellsMixin.type.ts(2 hunks)packages/core/src/view/plugins/CellEditorHandler.ts(9 hunks)packages/core/src/view/style/Stylesheet.ts(3 hunks)packages/html/stories/OrgChart.stories.js(3 hunks)packages/html/stories/Scrollbars.stories.js(1 hunks)packages/html/stories/SecondLabel.stories.js(1 hunks)
💤 Files with no reviewable changes (1)
- packages/core/src/util/Constants.ts
🧰 Additional context used
🧬 Code Graph Analysis (6)
packages/core/src/view/geometry/node/TextShape.ts (1)
packages/core/src/types.ts (2)
AlignValue(924-924)VAlignValue(926-926)
packages/core/src/view/mixins/CellsMixin.type.ts (1)
packages/core/src/types.ts (2)
AlignValue(924-924)VAlignValue(926-926)
packages/core/src/util/styleUtils.ts (1)
packages/core/src/types.ts (2)
AlignValue(924-924)VAlignValue(926-926)
packages/core/src/view/mixins/CellsMixin.ts (1)
packages/core/src/util/Constants.ts (1)
DEFAULT_IMAGESIZE(362-362)
packages/core/src/view/geometry/node/LabelShape.ts (1)
packages/core/src/util/Constants.ts (1)
DEFAULT_IMAGESIZE(362-362)
packages/core/src/view/geometry/stencil/StencilShape.ts (1)
packages/core/src/types.ts (2)
AlignValue(924-924)VAlignValue(926-926)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build
- GitHub Check: build (ubuntu-22.04)
- GitHub Check: build (macos-14)
- GitHub Check: build (windows-2022)
🔇 Additional comments (76)
packages/html/stories/Scrollbars.stories.js (1)
503-504: Correctly replaced enum with string literal.The
constants.ALIGN.TOPenum has been properly replaced with the string literal'top'for theverticalAlignproperty, aligning with the PR's objective to remove theALIGNenum.packages/core/src/view/cell/CellState.ts (2)
26-26: Correctly updated import statement.The import statement has been updated to remove
ALIGNfrom the imports, keeping onlyNONEfrom the Constants file, which is still being used in theisTransparentStatemethod.
403-403: Correctly replaced enum with string literal default value.The default vertical alignment value has been changed from
ALIGN.MIDDLEto the string literal'middle', which is consistent with the PR's objective to remove theALIGNenum.packages/html/stories/OrgChart.stories.js (3)
105-106: Correctly replaced style enums with string literals.The vertical alignment and horizontal alignment properties have been properly updated to use string literals (
'middle'and'left') instead of enum constants.
315-316: Correctly updated overlay alignment property.The
overlay.alignproperty has been updated to use the string literal'center'rather than the enum constant.
326-327: Successfully replaced overlay alignment properties.Both alignment properties for the delete icon overlay have been updated to use string literals (
'right'and'top') instead of their enum constant equivalents.CHANGELOG.md (2)
14-14: Appropriate documentation of breaking change for ALIGN enum.The changelog correctly documents that
constants.ALIGNhas been removed and replaced with the string literal typesAlignValueandVAlignValue, providing clear guidance for users who need to migrate.
18-18: Clear documentation for RENDERING_HINT removal.The changelog appropriately notes that
constants.RENDERING_HINThas been removed with no replacement provided since it wasn't used in the codebase.packages/core/src/view/style/Stylesheet.ts (3)
19-19: Updated import to remove unused ALIGN constantThe import statement has been correctly modified to only import the
NONEconstant, removing the no-longer-neededALIGNconstant in line with the PR objective of removing the ALIGN enum.
70-71: Replaced ALIGN enum with string literalsThe default vertex style correctly uses string literals
'middle'and'center'for theverticalAlignandalignproperties instead of the previous enum values. This is consistent with the change to useAlignValueandVAlignValuetypes throughout the codebase.
86-87: Replaced ALIGN enum with string literals in edge styleSimilar to the vertex style, the default edge style now correctly uses string literals for alignment values instead of enum references, maintaining consistency across the codebase.
packages/core/src/serialization/ObjectCodec.ts (3)
177-177: Updated code example to remove reference to ALIGN constantThe documentation example has been appropriately updated to reference a generic
MyConstant.PROPinstead of the removedconstants.ALIGNconstant.
181-181: Updated XML example to match new constant exampleThe XML example has been updated to be consistent with the new constant example, removing any reference to the removed ALIGN enum.
185-186: Updated explanation text to match new exampleThe explanation text now correctly references the expected output of "myValue" to match the updated example using
MyConstant.PROP. This maintains consistency in the documentation.packages/html/stories/SecondLabel.stories.js (1)
124-124: Replaced ALIGN constants with string literals in TextShape instantiationThe TextShape constructor now directly uses string literals
'left'and'bottom'instead of the previous enum constants. This aligns with the PR objective of removing the ALIGN enum from the codebase.packages/core/src/view/geometry/node/TextShape.ts (6)
68-69: Updated constructor default parameters to use string literalsThe constructor default parameters for
alignandvalignnow use string literals ('center'and'middle') instead of enum values, properly typed withAlignValueandVAlignValuetypes.
93-94: Updated property initialization to use string literalsProperty initialization in the constructor now consistently uses string literals instead of enum values for alignment properties.
346-347: Updated resetStyles method to use string literalsThe
resetStylesmethod now correctly uses string literals for resetting alignment properties, maintaining consistency with the constructor and the rest of the codebase.
448-449: Updated variable initialization and conditional check to use string literalsLocal variable defaults and conditional checks in the
updateBoundingBoxmethod now use string literals instead of enum references, properly handling the center/middle alignment case for clipped strings.Also applies to: 455-455
815-817: Updated conditional logic to use string literals in updateFontThe conditional checks in the
updateFontmethod now compare against string literals rather than enum values, maintaining the same logical flow for text alignment.
905-918: Updated spacing calculation to use string literalsThe conditional blocks in the
getSpacingmethod now compare against string literals instead of enum values, ensuring spacing calculations remain consistent with the previous implementation.packages/core/src/view/geometry/stencil/StencilShape.ts (1)
557-558: Updated text node attributes to use string literals with type annotationsThe alignment and vertical alignment attributes for text nodes are now correctly retrieved using the nullish coalescing operator with string literals (
'left'and'top') as fallbacks, properly typed asAlignValueandVAlignValue. This change is consistent with the removal of the ALIGN enum.packages/core/src/editor/Editor.ts (13)
36-36: Import statement updated to remove ALIGN enum referenceThe import statement has been updated to only import
FONTfrom Constants, removing theALIGNenum import as part of the refactoring to eliminate this enum.
1161-1162: String literal used instead of ALIGN enum valueChanged from using
ALIGN.LEFTenum value to the string literal'left'in thealignCellsmethod call, aligning with the enum removal refactoring.
1167-1168: String literal used instead of ALIGN enum valueChanged from using
ALIGN.CENTERenum value to the string literal'center'in thealignCellsmethod call.
1173-1174: String literal used instead of ALIGN enum valueChanged from using
ALIGN.RIGHTenum value to the string literal'right'in thealignCellsmethod call.
1179-1180: String literal used instead of ALIGN enum valueChanged from using
ALIGN.TOPenum value to the string literal'top'in thealignCellsmethod call.
1185-1186: String literal used instead of ALIGN enum valueChanged from using
ALIGN.MIDDLEenum value to the string literal'middle'in thealignCellsmethod call.
1191-1192: String literal used instead of ALIGN enum valueChanged from using
ALIGN.BOTTOMenum value to the string literal'bottom'in thealignCellsmethod call.
1196-1196: String literal used instead of ALIGN enum valueChanged from using
ALIGN.LEFTenum value to the string literal'left'in thesetCellStylesmethod call for the align property.
1201-1202: String literal used instead of ALIGN enum valueChanged from using
ALIGN.CENTERenum value to the string literal'center'in thesetCellStylesmethod call for the align property.
1207-1208: String literal used instead of ALIGN enum valueChanged from using
ALIGN.RIGHTenum value to the string literal'right'in thesetCellStylesmethod call for the align property.
1213-1214: String literal used instead of ALIGN enum valueChanged from using
ALIGN.TOPenum value to the string literal'top'in thesetCellStylesmethod call for the verticalAlign property.
1219-1220: String literal used instead of ALIGN enum valueChanged from using
ALIGN.MIDDLEenum value to the string literal'middle'in thesetCellStylesmethod call for the verticalAlign property.
1225-1226: String literal used instead of ALIGN enum valueChanged from using
ALIGN.BOTTOMenum value to the string literal'bottom'in thesetCellStylesmethod call for the verticalAlign property.packages/core/src/view/mixins/CellsMixin.type.ts (3)
19-25: Import AlignValue and VAlignValue types for type safetyAdded imports for
AlignValueandVAlignValuetype definitions, which are string literal union types that replace the previous enum values. This provides better type safety and ensures that only valid alignment values can be used.
246-246: Simplified align parameter documentationThe JSDoc comment for the
alignparameter has been simplified to remove the reference to theALIGNenum since it's being removed.
250-254: Updated alignCells method signature with stronger typingChanged the type of the
alignparameter from a genericstringto the more specificAlignValue | VAlignValueunion type. This improves type safety by restricting the allowed values to the predefined set of valid alignment strings.packages/core/src/view/cell/CellRenderer.ts (5)
359-359: String literal used instead of ALIGN enum valueChanged the default alignment from
ALIGN.CENTERenum value to the string literal'center'in the TextShape constructor call.
1032-1035: String literals used instead of ALIGN enum values in label position handlingReplaced
ALIGN.CENTERandALIGN.MIDDLEenum values with string literals'center'and'middle'in the label position and vertical label position handling. This maintains the same functionality while removing the dependency on the ALIGN enum.
1074-1076: String literals used instead of ALIGN enum valuesReplaced references to ALIGN enum constants with string literals for label position handling, while also using the local textShape variable for cleaner code access.
1081-1089: Improved code readability with consistent variable usageContinued use of the
textShapelocal variable along with string literals to replace ALIGN enum references, making the code more consistent and slightly more efficient by reducing repeated property access.
1093-1093: Consistent use of local variable for method callUsed the
textShapelocal variable for thegetTextRotation()method call, maintaining consistency with the other changes in this function.packages/core/src/view/GraphView.ts (3)
25-25: Updated import statement to remove ALIGN constant.The import statement has been updated to remove the ALIGN constant as part of the broader refactoring to replace enum constants with string literals.
1016-1018: String literals now used for horizontal alignment instead of enum.The code now uses string literals ('left', 'center', 'right') for horizontal alignment instead of the ALIGN enum, improving type safety with the AlignValue type.
1053-1061: String literals now used for vertical alignment instead of enum.The code now uses string literals ('top', 'middle', 'bottom') for vertical alignment instead of the ALIGN enum, improving type safety with the VAlignValue type.
packages/core/src/view/geometry/node/LabelShape.ts (8)
20-20: Updated import statement to remove ALIGN constant.Simplified import statement to remove the unused ALIGN constant while keeping the necessary DEFAULT_IMAGESIZE and NONE constants.
163-164: String literals now used for default alignment values.Default values for imageAlign and verticalAlign are now set to string literals 'left' and 'middle' instead of using the ALIGN enum constants.
169-176: Updated horizontal alignment checks to use string literals.Horizontal alignment comparisons now use string literals ('center', 'right', 'left') instead of the ALIGN enum constants, improving type safety with the AlignValue type.
178-185: Updated vertical alignment checks to use string literals.Vertical alignment comparisons now use string literals ('top', 'bottom', 'middle') instead of the ALIGN enum constants, improving type safety with the VAlignValue type.
192-192: Updated JSDoc parameter type annotation.The JSDoc parameter type for paintIndicator's 'c' parameter has been updated from '{mxAbstractCanvas2D}' to '{AbstractCanvas2D}' to align with the actual type used.
226-227: String literals now used for default alignment values in getIndicatorBounds.Default values for imageAlign and verticalAlign in getIndicatorBounds are now set to string literals 'left' and 'middle' instead of using the ALIGN enum constants.
232-239: Updated horizontal alignment checks in getIndicatorBounds.Horizontal alignment comparisons in getIndicatorBounds now use string literals ('right', 'center', 'left') instead of the ALIGN enum constants.
241-248: Updated vertical alignment checks in getIndicatorBounds.Vertical alignment comparisons in getIndicatorBounds now use string literals ('bottom', 'top', 'middle') instead of the ALIGN enum constants.
packages/core/src/view/mixins/CellsMixin.ts (7)
31-31: Simplified imports by removing ALIGN constant.The import statement has been updated to remove the ALIGN constant while keeping other necessary constants.
378-392: Updated alignment handling in alignCells method.The alignCells method now uses string literals ('center', 'right', 'top', 'middle', 'bottom') for alignment comparisons instead of the ALIGN enum constants, improving type safety with the AlignValue and VAlignValue types.
422-434: String literals used for alignment in geometry calculations.Alignment comparisons when setting cell geometries now use string literals ('center', 'right', 'top', 'middle', 'bottom') instead of the ALIGN enum constants.
902-908: Updated horizontal alignment handling in cellSizeUpdated.The cellSizeUpdated method now uses string literals for alignment, with a default of 'center' rather than ALIGN.CENTER, and proper string literal comparisons for alignment checks.
912-916: Updated vertical alignment handling in cellSizeUpdated.Vertical alignment comparisons in cellSizeUpdated now use string literals ('bottom', 'middle') instead of the ALIGN enum constants.
956-958: Updated vertical alignment check in getPreferredSizeForCell.The vertical alignment check in getPreferredSizeForCell now uses the string literal 'middle' instead of ALIGN.MIDDLE.
960-962: Updated alignment check in getPreferredSizeForCell.The alignment comparison in getPreferredSizeForCell now uses the string literal 'center' instead of ALIGN.CENTER.
packages/core/src/util/styleUtils.ts (4)
20-21: Simplified imports by removing ALIGN constant.The import statement has been updated to remove the ALIGN constant while keeping other necessary constants.
27-33: Added specific alignment type imports.The import statement now includes AlignValue and VAlignValue types, providing stronger type checking for alignment values throughout the code.
523-524: Updated function signature and documentation.The getAlignmentAsPoint function signature now uses the more specific AlignValue and VAlignValue types instead of generic strings, and the documentation has been updated to reflect the correct default values of center and middle.
529-540: Updated alignment comparisons in getAlignmentAsPoint.The alignment comparisons in getAlignmentAsPoint now use string literals ('left', 'right', 'top', 'bottom') instead of the ALIGN enum constants, making the code more consistent with the string-based approach used throughout the codebase.
packages/core/src/view/plugins/CellEditorHandler.ts (4)
51-51: AddAlignValueandGraphPluginto type imports
This ensures the file now explicitly uses the refinedAlignValueandGraphPlugintypes instead of the removedALIGNenum.
163-163: Prefix unused parameter with underscore inchangeHandler
Renamingsenderto_sendermakes it clear the parameter is intentionally unused and silences unused-variable warnings.
278-278: UseAlignValuetype foralignproperty
Refining the property type from a genericstring | nulltoAlignValue | nullimproves compile-time type checking.
308-308: UpdatesetAlignsignature to acceptAlignValue
Aligns the method signature with the new literal type and removes any remaining coupling to the old enum.packages/core/src/view/canvas/SvgCanvas2D.ts (7)
262-262: Refinevalignparameter toVAlignValue
Switching from a genericstringto the specificVAlignValuetightens the API and catches invalid values earlier.
281-281: MapAlignValuedirectly to CSStext-align
The updated ternaryalign === 'left' ? 'left' : align === 'right' ? 'right' : 'center'correctly covers all literal cases.
463-464: Use string literal cases for foreignObject fallback
Mappingvalignandaligndirectly to numeric offsets and SVGtext-anchorvalues simplifies logic now that enums are gone.
1515-1517: Adjust horizontal clip offset using literalalign
Replacing enum checks with string-literal conditions ensures the clip rectangle is correctly centered or right-aligned.
1522-1524: Adjust vertical clip offset using literalvalign
The new comparison for'middle'correctly applies a half-height shift when clipping non-fill text.
1558-1558: Determine default SVGtext-anchorfromalignliteral
The expressionalign === 'right' ? 'end' : align === 'center' ? 'middle' : 'start'matches SVG defaults without enum indirection.
1735-1737: Refactor background bounds alignment using literalalign/valign
The x/y shifts for center/right and middle/bottom in the background rectangle mirror the text placement; please verify these calculations to avoid off-by-one pixel misalignments.Also applies to: 1741-1743
|
BREAKING CHANGES:
constants.ALIGNenum has been removed, useAlignValueandVAlignValuetypes insteadconstants.RENDERING_HINTenum has been removed without replacement because it wasn't usedNotes
Covers #378
Note
The SonarQube check fails because it detects too many duplication.
The duplication already exists and is not introduced by this PR
Summary by CodeRabbit
Breaking Changes
Refactor
Documentation