refactor(image): [image] refactor the image theme #2236
Conversation
WalkthroughThe pull request introduces modifications to the Playwright testing suite for the image viewer component, specifically updating expected CSS property values related to the visual style of a mask element. Additionally, significant changes are made to the styling properties in the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (4)
examples/sites/demos/pc/app/image/keep-style.spec.ts (1)
21-21: LGTM. Consider adding a comment for clarity.The change to the expected background color of the mask element from 'rgba(0, 0, 0, 0.5)' to 'rgba(0, 0, 0, 0.08)' appears to be intentional and aligns with the refactoring of the image theme. This reduces the opacity of the mask from 50% to 8%, making it less opaque.
Consider adding a brief comment explaining the reason for this specific color change, which could help future maintainers understand the design decision. For example:
// Mask opacity reduced to 8% for a lighter overlay effect await expect(mask).toHaveCSS('background-color', 'rgba(0, 0, 0, 0.08)')packages/theme/src/image/index.less (1)
Line range hint
1-124: Summary of changes and potential impactThe modifications in this file are part of the larger refactoring effort for the image theme. The changes replace CSS variable usage with fixed
inline-blockdisplay values, which promotes consistency but may reduce theme customizability.Consider the following points for the overall refactoring process:
- Ensure that removing CSS variables aligns with the project's theming strategy and doesn't negatively impact existing theme configurations.
- Look for opportunities to refactor common styles to improve maintainability, as suggested in the previous comment.
- Document these changes in the component's documentation, especially if they affect public APIs or customization options.
- Update any related test cases to reflect these new fixed display values.
These considerations will help maintain the balance between consistency and flexibility in the component's styling.
packages/theme/src/image/vars.less (2)
15-15: Approved: Font size variable updated for consistencyThe font size variable has been updated to use a more standardized naming convention (
--tv-font-size-md). This change likely improves consistency across the theme system.Ensure that
--tv-font-size-mdis properly defined and its value is appropriate for the error text in the image component.
19-19: Approved: Background color variable updated for consistencyThe background color variable has been updated to use a more standardized naming convention (
--tv-color-bg). This change improves consistency across the theme system.The new variable
--tv-color-bgseems more generic than the previous one. Ensure that this doesn't unintentionally affect the visual design of the error state in the image component.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- examples/sites/demos/pc/app/image/keep-style.spec.ts (1 hunks)
- examples/sites/demos/pc/app/image/preview.spec.ts (1 hunks)
- packages/theme/src/image-viewer/index.less (5 hunks)
- packages/theme/src/image-viewer/vars.less (1 hunks)
- packages/theme/src/image/index.less (1 hunks)
- packages/theme/src/image/vars.less (1 hunks)
🧰 Additional context used
🔇 Additional comments (21)
examples/sites/demos/pc/app/image/preview.spec.ts (1)
21-21: LGTM. Please verify color consistency and document the change.The update to the expected background color aligns with the PR's objective of refactoring the image theme. However, to ensure completeness:
- Please verify that this new color value (rgba(0, 0, 0, 0.08)) is consistent with other parts of the codebase and the new theme.
- Consider adding a brief comment explaining the reason for this specific color change, as it's a significant reduction in opacity (from 0.5 to 0.08).
To verify color consistency, you can run:
examples/sites/demos/pc/app/image/keep-style.spec.ts (1)
Line range hint
1-52: Overall, the test case looks well-structured and comprehensive.The test case covers various aspects of the image viewer component, including:
- Initial rendering and styling
- Zoom in/out functionality
- Rotation
- Navigation between images
- Closing the viewer
The change to the mask's background color is the only modification, and it has been appropriately updated in the test. The rest of the test case remains robust and should continue to provide good coverage for the component's functionality.
packages/theme/src/image/index.less (1)
51-51: Verify the impact of removing the CSS variable for display propertyThe change from
var(--ti-image-error-display)toinline-blockfor the error text display property removes the flexibility provided by the CSS variable. While this ensures consistent behavior, it might limit theme customization.Please confirm if this change aligns with the refactoring goals and doesn't negatively impact any existing theme configurations. Consider the following script to check for any other usages of the removed variable:
packages/theme/src/image/vars.less (3)
15-21: Overall: Significant updates to theming variablesThe changes in this file represent a shift towards a more standardized theming system. Here are the key points:
- All variables now use the
--tv-prefix instead of--ti-common-, indicating a move to a new naming convention.- Fallback values have been removed, increasing reliance on the new theming system.
- The
--ti-image-error-displayvariable has been removed entirely.These changes likely improve the consistency and maintainability of the theming system. However, they also introduce some potential risks:
- Ensure that all new variables (
--tv-font-size-md,--tv-color-text-weaken,--tv-color-bg,--tv-line-height-number) are properly defined in the new theming system.- Verify that the removal of fallback values doesn't cause issues in environments where the new theme variables might not be available.
- Check if the removal of
--ti-image-error-displayaffects the component's behavior or if this property is now controlled elsewhere.To ensure these changes don't introduce unintended side effects, consider running visual regression tests on the image component, particularly focusing on its error state display.
21-21: Approved: Line height variable updated for consistencyThe line height variable has been updated to use a more standardized naming convention (
--tv-line-height-number). This change improves consistency across the theme system.Verify that
--tv-line-height-numberis properly defined and its value is appropriate for the error text:#!/bin/bash # Search for the definition of --tv-line-height-number rg --type css "tv-line-height-number"
17-17: Approved: Text color variable updated for consistencyThe text color variable has been updated to use a more standardized naming convention (
--tv-color-text-weaken). This change improves consistency across the theme system.Verify that
--tv-color-text-weakenis properly defined:✅ Verification successful
Verified:
--tv-color-text-weakenis properly definedThe
--tv-color-text-weakenvariable is correctly defined inpackages/theme/src/base/vars.lessand is consistently used across other theme files. The update inpackages/theme/src/image/vars.lessto use this standardized variable ensures consistency within the theme system.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the definition of --tv-color-text-weaken rg --type css "tv-color-text-weaken"Length of output: 37
Script:
#!/bin/bash # Search for the definition of --tv-color-text-weaken in LESS files rg --type less "--tv-color-text-weaken"Length of output: 464
Script:
#!/bin/bash # Search for the definition of --tv-color-text-weaken in LESS files rg --type less "--tv-color-text-weaken"Length of output: 464
Script:
#!/bin/bash # Search for the definition of --tv-color-text-weaken in LESS files rg --type less -- "--tv-color-text-weaken"Length of output: 1251
packages/theme/src/image-viewer/vars.less (2)
14-27: Ensure All New Variables Are DefinedVerify that all the new variables prefixed with
--tv-are defined in the theming system and that they map to the correct values. This helps prevent runtime errors due to undefined variables and ensures consistency across components.You can run the following script to confirm that all variables are defined:
#!/bin/bash # Description: Check for undefined --tv- variables in the theme source files # Test: List all --tv- variables and check their definitions rg --no-heading --line-number '--tv-[a-zA-Z0-9\-]+' packages/theme/src/ | sort | uniq
17-17: Verify the Variable--tv-color-bg-control-unactiveEnsure that the variable
--tv-color-bg-control-unactiveis defined in your theming system and correctly references the intended background color. If it's a new variable, make sure it's added to the appropriate variables file.Run the following script to check if the variable is defined elsewhere in the codebase:
✅ Verification successful
Reattempt Verification for
--tv-color-bg-control-unactiveThe previous script encountered a syntax error. Please run the corrected script below to verify if the variable is defined.
Variable
--tv-color-bg-control-unactiveis Properly DefinedThe variable is defined in
packages/theme/src/base/vars.lessand used appropriately inpackages/theme/src/image-viewer/vars.less.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for the definition of --tv-color-bg-control-unactive # Test: Find the variable definition in the theme source files rg --no-heading --line-number '--tv-color-bg-control-unactive' packages/theme/src/Length of output: 509
Script:
#!/bin/bash # Description: Search for the definition of --tv-color-bg-control-unactive # Corrected command with proper quoting rg -e '--tv-color-bg-control-unactive' packages/theme/src/Length of output: 298
packages/theme/src/image-viewer/index.less (13)
23-23: LGTMSetting the background of
&__wrappertononeis appropriate to remove any default background styling, ensuring transparency and relying on parent elements for background properties.
38-38: LGTMAssigning
opacity: 1to&__btnensures that the button is fully opaque, which is suitable for visibility and user interaction.
43-45: LGTMDefining explicit
width,height, andfillproperties for the SVG within&__btnenhances consistency and ensures that the icons display correctly across different environments.
50-55: LGTMUpdating the positioning, dimensions, font size, and background color of
&__closeusing CSS variables improves maintainability and theming flexibility.
63-63: LGTMAdding the
&__canvasclass enhances the component structure, providing a dedicated area for rendering the image content.
73-79: LGTMUpdating
&__actionswith new positioning, dimensions, padding, background color, and border-radius using CSS variables enhances theming capabilities and aligns with design standards.
88-88: LGTMChanging
justify-contenttospace-aroundin&__actions-innerensures that the action items are evenly distributed, improving the user interface and interaction.
91-92: LGTMApplying CSS variables for
fillandmargin-rightin SVG icons within&__actions-innerpromotes consistency and eases theme customization.
101-101: LGTMSetting
display: inline-blockfor&__actions-divideris appropriate for aligning dividers correctly alongside inline elements.
105-109: LGTMDefining
display, dimensions,line-height, andfont-sizefor&__actions-countusing CSS variables improves consistency and adaptability to different themes.
117-120: LGTMUpdating
&__nextand&__prevwith dimensions and font sizes using CSS variables enhances theming flexibility and maintains consistent styling across navigation elements.
130-130: LGTMUsing a CSS variable for
leftpositioning in&__prev(var(--tv-Image-viewer-prev-left)) enhances flexibility and consistency, allowing for easy adjustments through theming.
135-135: LGTMApplying a CSS variable for
rightpositioning in&__next(var(--tv-Image-viewer-next-right)) improves maintainability and enables streamlined theme customization.
| --tv-Image-viewer-next-font-size: calc(var(--tv-size-base) * 6); | ||
| --tv-Image-viewer-next-width: var(--tv-size-height-lg); | ||
| --tv-Image-viewer-next-height: var(--tv-size-height-lg); | ||
| --tv-Image-viewer-btn-opacity: 1; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider Using a Variable for Opacity
The opacity value is set directly to 1. If you anticipate needing different opacity levels for theming purposes, consider defining a CSS variable for the opacity value.
Example:
- --tv-Image-viewer-btn-opacity: 1;
+ --tv-Image-viewer-btn-opacity: var(--tv-button-opacity, 1);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --tv-Image-viewer-btn-opacity: 1; | |
| --tv-Image-viewer-btn-opacity: var(--tv-button-opacity, 1); |
|
|
||
| &:hover { | ||
| cursor: pointer; | ||
| background-color: var(--ti-image-viewer-close-bg-color-hover); | ||
| background-color: #606266; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use CSS variable instead of hardcoded color in hover state
In the &__close element's :hover state, the background-color is set to a hardcoded value #606266. To maintain consistency with the theming system and facilitate easier customization, consider replacing the hardcoded color with a CSS variable.
Apply this diff to replace the hardcoded color:
&__close {
// existing styles
&:hover {
cursor: pointer;
- background-color: #606266;
+ background-color: var(--tv-Image-viewer-close-hover-bg-color);
}
}Committable suggestion was skipped due to low confidence.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (3)
packages/theme/src/image/vars.less (2)
23-23: LGTM! Consider externalizing the data URL.The update of
--ti-image-error-bg-image-urlto--tv-Image-error-bg-image-urlis consistent with the other changes. However, the long data URL might impact readability and maintainability of this file.Consider moving the data URL to a separate file (e.g., a JavaScript module) and importing it here. This would improve the readability of this file and make it easier to update the image in the future.
13-23: Overall, good refactoring towards a more consistent theming system.The changes in this file represent a significant step towards a more consistent and modular theming system. The new naming conventions and use of shared variables should improve maintainability. However, please ensure that all dependent components are updated accordingly and consider the suggestions for further improvements.
As you continue this refactoring effort, consider creating a style guide or documentation for the new theming system to ensure consistency across the entire project.
packages/theme/src/image-viewer/index.less (1)
38-38: Nitpick: Explicitopacity: 1;may be unnecessarySince the default value of
opacityis1, explicitly settingopacity: 1;might be redundant unless it's intended to override a previous styling. Consider removing it if it's not necessary.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- packages/theme/src/image-viewer/index.less (5 hunks)
- packages/theme/src/image-viewer/vars.less (1 hunks)
- packages/theme/src/image/index.less (3 hunks)
- packages/theme/src/image/vars.less (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/theme/src/image/index.less
🧰 Additional context used
🔇 Additional comments (6)
packages/theme/src/image/vars.less (3)
15-21: LGTM! Update related components.The CSS variable updates look good. The new naming convention with the
--tv-prefix is more consistent and the use of other--tv-prefixed variables for values suggests a more modular approach. This change improves maintainability and readability of the code.Please ensure that all components using these variables are updated accordingly. Run the following script to find potential places that need updates:
#!/bin/bash # Search for any remaining usage of the old variable names rg --type less "\-\-ti-(image-error-font-size|image-error-text-color|image-error-bg-color|image-error-line-height)"
13-13: LGTM! Verify usage in other files.The function name change from
.component-css-vars-image()to.inject-Image-vars()looks good. It appears to follow a new naming convention, which is more consistent with modern CSS-in-JS practices.Please run the following script to check for any remaining usages of the old function name:
13-23: Clarify removal of--ti-image-error-display.The variable
--ti-image-error-displayhas been removed. Can you please clarify the reason for this removal? It's important to ensure that this change doesn't negatively impact the error state handling in the image component.To help understand the impact, please run the following script to find any remaining usage of this variable:
✅ Verification successful
Re-verify usage of
--ti-image-error-display.To ensure that the variable has been fully removed and is not used elsewhere in the codebase, please run the following script:
Re-verify usage of
--ti-image-error-displaywith corrected search command.To ensure that the variable has been fully removed and is not used elsewhere in the codebase, please run the following corrected script:
Re-verify usage of
--ti-image-error-displaywith corrected search command.To ensure that the variable has been fully removed and is not used elsewhere in the codebase, please run the following corrected script:
Confirmed removal of
--ti-image-error-display.No remaining usages of
--ti-image-error-displaywere found in the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining usage of the removed variable rg --type less "\-\-ti-image-error-display"Length of output: 45
Script:
#!/bin/bash # Search for any remaining usage of the removed variable across all files rg '--ti-image-error-display'Length of output: 456
Script:
#!/bin/bash # Search for any remaining usage of the removed variable across all files rg '--ti-image-error-display'Length of output: 456
Script:
#!/bin/bash # Search for any remaining usage of the removed variable across all files rg -- '--ti-image-error-display'Length of output: 32
packages/theme/src/image-viewer/vars.less (1)
20-20: Verify Unit Usage in CalculationThe calculation
calc(var(--tv-size-base) * 6)may not work as expected if--tv-size-basedoes not include units. In CSS, when usingcalc()for multiplication, at least one operand must have units.Please ensure that
--tv-size-baseincludes units (e.g.,1rem,16px). If it doesn't, you might need to adjust the calculation.If
--tv-size-baseis unitless, consider updating the calculation:--tv-ImageViewer-next-font-size: calc(var(--tv-size-base) * 6); --tv-ImageViewer-next-font-size: calc(6 * 1rem);Run the following script to verify the definition of
--tv-size-base:✅ Verification successful
Unit Usage Verified for
--tv-size-baseThe
--tv-size-basevariable is defined with units (4px), ensuring that the calculationcalc(var(--tv-size-base) * 6)will work as expected.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if `--tv-size-base` includes units. # Search for the definition of `--tv-size-base` in the codebase. rg --type less --no-filename --no-line-number --word-regexp --fixed-strings '--tv-size-base:'Length of output: 511
Script:
#!/bin/bash # Description: Verify if `--tv-size-base` includes units. # Corrected command: Ensure the pattern is correctly passed as a search string. rg --type less --no-filename --no-line-number --word-regexp --fixed-strings '--tv-size-base:' .Length of output: 513
Script:
#!/bin/bash # Description: Verify if `--tv-size-base` includes units using the -e flag to specify the pattern. rg --type less --no-filename --no-line-number --word-regexp --fixed-strings -e '--tv-size-base:' .Length of output: 121
packages/theme/src/image-viewer/index.less (2)
23-23: Verify that settingbackground: none;does not impact themingBy replacing the background variable with
background: none;in&__wrapper, there might be unintended side effects on theming and customization. Please ensure that this change does not adversely affect the ability to theme or customize the background of the image viewer component.
88-88: Verify thatjustify-content: space-around;achieves the desired layoutChanging
justify-contenttospace-aroundmay alter the spacing between elements in&__actions-inner. Please confirm that this modification aligns with the intended design and does not negatively impact the user interface.
| --tv-ImageViewer-text-color: var(--tv-color-success-text-white); | ||
| --tv-ImageViewer-actions-inner-text-color: var(--tv-color-success-text-white); | ||
| --tv-ImageViewer-close-font-size: var(--tv-font-size-xxl); | ||
| --tv-ImageViewer-close-bg-color: var(--tv-color-bg-control-unactive); |
There was a problem hiding this comment.
Typo in Variable Name Reference
The variable name --tv-color-bg-control-unactive may contain a typo. The term "unactive" is uncommon in English; perhaps you intended to use "inactive".
Apply this diff to correct the variable name reference:
--tv-ImageViewer-close-bg-color: var(--tv-color-bg-control-unactive);
--tv-ImageViewer-close-bg-color: var(--tv-color-bg-control-inactive);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --tv-ImageViewer-close-bg-color: var(--tv-color-bg-control-unactive); | |
| --tv-ImageViewer-close-bg-color: var(--tv-color-bg-control-inactive); |
| .inject-ImageViewer-vars() { | ||
| --tv-ImageViewer-text-color: var(--tv-color-success-text-white); | ||
| --tv-ImageViewer-actions-inner-text-color: var(--tv-color-success-text-white); | ||
| --tv-ImageViewer-close-font-size: var(--tv-font-size-xxl); | ||
| --tv-ImageViewer-close-bg-color: var(--tv-color-bg-control-unactive); | ||
| --tv-ImageViewer-actions-border-radius: var(--tv-border-radius-round); | ||
| --tv-ImageViewer-actions-height: var(--tv-size-height-lg); | ||
| --tv-ImageViewer-next-font-size: calc(var(--tv-size-base) * 6); | ||
| --tv-ImageViewer-next-width: var(--tv-size-height-lg); | ||
| --tv-ImageViewer-next-height: var(--tv-size-height-lg); | ||
| --tv-ImageViewer-actions-icon-margin-right: var(--tv-space-xl); | ||
| --tv-ImageViewer-actions-count-font-size: var(--tv-font-size-md); | ||
| --tv-ImageViewer-prev-left: calc(var(--tv-space-base) * 8); | ||
| --tv-ImageViewer-next-right: calc(var(--tv-space-base) * 8); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consistent Naming Convention for CSS Variables
The CSS custom properties are using PascalCase with mixed capitalization, e.g., --tv-ImageViewer-text-color. For consistency and to align with standard CSS practices, it's recommended to use all lowercase letters with hyphens.
Consider updating the variable names to use all lowercase letters with hyphens:
- --tv-ImageViewer-text-color: var(--tv-color-success-text-white);
+ --tv-image-viewer-text-color: var(--tv-color-success-text-white);Apply similar changes to all custom properties within this block.
Committable suggestion was skipped due to low confidence.
| width: 20px; | ||
| height: 20px; | ||
| line-height: 20px; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using CSS variables for dimensions
To enhance theming capabilities and maintain consistency, consider replacing hardcoded values for width, height, and line-height with CSS variables. This will allow for easier customization and scalability.
Example modifications:
- width: 20px;
+ width: var(--tv-ImageViewer-actions-count-width, 20px);
- height: 20px;
+ height: var(--tv-ImageViewer-actions-count-height, 20px);
- line-height: 20px;
+ line-height: var(--tv-ImageViewer-actions-count-line-height, 20px);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| width: 20px; | |
| height: 20px; | |
| line-height: 20px; | |
| width: var(--tv-ImageViewer-actions-count-width, 20px); | |
| height: var(--tv-ImageViewer-actions-count-height, 20px); | |
| line-height: var(--tv-ImageViewer-actions-count-line-height, 20px); |
| width: 248px; | ||
| height: var(--tv-ImageViewer-actions-height); | ||
| padding: 10px 24px; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using CSS variables for width and padding
To maintain consistency with the theming system and allow for greater flexibility, consider replacing the hardcoded width and padding values with CSS variables.
Suggested changes:
- width: 248px;
+ width: var(--tv-ImageViewer-actions-width, 248px);
- padding: 10px 24px;
+ padding: var(--tv-ImageViewer-actions-padding, 10px 24px);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| width: 248px; | |
| height: var(--tv-ImageViewer-actions-height); | |
| padding: 10px 24px; | |
| width: var(--tv-ImageViewer-actions-width, 248px); | |
| height: var(--tv-ImageViewer-actions-height); | |
| padding: var(--tv-ImageViewer-actions-padding, 10px 24px); |
| top: 32px; | ||
| right: 32px; | ||
| width: 40px; | ||
| height: 40px; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using CSS variables for positioning and dimensions
To enhance theming flexibility and maintain consistency, consider replacing hardcoded pixel values for top, right, width, and height with CSS variables. This allows for easier customization and responsiveness across different screen sizes.
Example changes:
- top: 32px;
+ top: var(--tv-ImageViewer-close-top, 32px);
- right: 32px;
+ right: var(--tv-ImageViewer-close-right, 32px);
- width: 40px;
+ width: var(--tv-ImageViewer-close-width, 40px);
- height: 40px;
+ height: var(--tv-ImageViewer-close-height, 40px);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| top: 32px; | |
| right: 32px; | |
| width: 40px; | |
| height: 40px; | |
| top: var(--tv-ImageViewer-close-top, 32px); | |
| right: var(--tv-ImageViewer-close-right, 32px); | |
| width: var(--tv-ImageViewer-close-width, 40px); | |
| height: var(--tv-ImageViewer-close-height, 40px); |
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Bug Fixes
Documentation