fix(VE-6459): psuedo-editable height collapse#451
Conversation
The height of the cloned/psuedo-editable element collapses when text-wrap-mode is set to nowrap and text-overflow is set to ellipsis. This change ensures that we override these properties CORRECTLY. Previously we overrode these properties but only in camel case. Now we ensure we perform the camelCase transformation after overriding properties. We also set min-height to whatever the current height of the element is, to prevent height collapse in other scenarios.
faraazb
changed the title
hiteshshetty-dev
left a comment
hiteshshetty-dev
left a comment
There was a problem hiding this comment.
Can we please add test cases for these changes?
| if (camelCase) { | ||
| styles = getCamelCaseStyles(styles); | ||
| } |
There was a problem hiding this comment.
This will override all the styles if camelCase is true. Is this expected?
There was a problem hiding this comment.
Yes, this is expected. I moved this down so we camelCase both exising and overriden fields. However, we are dependent on camelCase implementation in this case, so I have now removed this dependency on the latest commit
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull Request Overview
This PR refactors how pseudo-editable element styles are computed by introducing a dedicated utility for essential overrides and ensuring camel-case transformation occurs after merging. It also adds a min-height override to prevent collapse when measuring height.
- Extract
getPsuedoEditableEssentialStylesto centralize overrides (position, wrapping, overflow, min-height). - Move camel-case conversion to after merging essential styles in
getPsuedoEditableElementStyles. - Update and expand tests to cover merging logic and style key transformations.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/visualBuilder/utils/getPsuedoEditableStylesElement.ts | Refactor to merge computed styles with essential overrides via new helper and apply camelCase last |
| src/visualBuilder/utils/getPsuedoEditableEssentialStyles.ts | New helper defining essential style overrides (position, wrapping, overflow, min-height) |
| src/visualBuilder/utils/test/getPsuedoEditableStylesElement.test.ts | Update tests to mock and verify merging and camelCase logic |
| src/visualBuilder/utils/test/getPsuedoEditableEssentialStyles.test.ts | Add tests covering kebab/camel-case outputs and scroll-affected positioning |
Comments suppressed due to low confidence (2)
src/visualBuilder/utils/getPsuedoEditableStylesElement.ts:1
- The term "Psuedo" is consistently misspelled; consider renaming functions, files, and folders from "Psuedo" to "Pseudo" to correct the spelling.
import getCamelCaseStyles from "./getCamelCaseStyles";
src/visualBuilder/utils/test/getPsuedoEditableStylesElement.test.ts:12
- Add an assertion to verify that
getCamelCaseStylesis not called whencamelCaseisfalseor undefined, to fully cover the flag’s behavior.
it("should return merged styles from getStyleOfAnElement and getPsuedoEditableEssentialStyles", () => {
hiteshshetty-dev
left a comment
hiteshshetty-dev
left a comment
There was a problem hiding this comment.
LGTM!!
Have we tested this on compass app?
Tested on Compass with different text elements, works fine. (See video attached in the JIRA ticket.) |
The height of the cloned/psuedo-editable element collapses when text-wrap-mode is set to nowrap and text-overflow is set to ellipsis. This change ensures that we override these properties CORRECTLY. Previously we overrode these properties but only in camel case. Now we ensure we perform the camelCase transformation after overriding properties. We also set min-height to whatever the current height of the element is, to prevent height collapse in other scenarios.