fix error fields style when underline is used as field shape type#1982
Conversation
WalkthroughThe changes introduce a new case, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FrmStylesHelper
participant CSS
User->>FrmStylesHelper: Trigger border width error
FrmStylesHelper->>CSS: Process border_width_error case
CSS-->>FrmStylesHelper: Return updated border width
FrmStylesHelper-->>User: Apply new border styling
Possibly related PRs
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 (
|
…gate when a the sized is changed only, fix error text description color
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (5)
js/form-templates.js.mapis excluded by!**/*.map,!**/*.mapjs/formidable_dashboard.js.mapis excluded by!**/*.map,!**/*.mapjs/formidable_overlay.js.mapis excluded by!**/*.map,!**/*.mapjs/formidable_styles.js.mapis excluded by!**/*.map,!**/*.mapjs/onboarding-wizard.js.mapis excluded by!**/*.map,!**/*.map
Files selected for processing (10)
- classes/helpers/FrmStylesHelper.php (2 hunks)
- classes/models/FrmStyle.php (4 hunks)
- classes/views/styles/_quick-settings.php (2 hunks)
- classes/views/styles/components/FrmSliderStyleComponent.php (5 hunks)
- classes/views/styles/components/FrmStyleComponent.php (1 hunks)
- classes/views/styles/components/templates/slider.php (9 hunks)
- css/_single_theme.css.php (2 hunks)
- css/admin/style-components.css (1 hunks)
- js/formidable_styles.js (1 hunks)
- js/src/admin/styles/slider-style-component.js (3 hunks)
Files skipped from review due to trivial changes (1)
- classes/models/FrmStyle.php
Files skipped from review as they are similar to previous changes (2)
- classes/helpers/FrmStylesHelper.php
- css/_single_theme.css.php
Additional context used
Biome
js/formidable_styles.js
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
Additional comments not posted (24)
classes/views/styles/components/FrmSliderStyleComponent.php (8)
36-36: LGTM!The assignment of
value_labelbased on the detected unit measurement ensures consistent formatting and aligns with the AI-generated summary.
57-57: LGTM!The inclusion of an empty string as the first element in the returned array when no units are provided allows for more flexible unit selection and aligns with the AI-generated summary.
94-94: LGTM!The assignment of the
valueproperty based on the detected unit measurement ensures consistent formatting and aligns with the AI-generated summary.
99-99: LGTM!The assignment of the
valueproperty based on the detected unit measurement ensures consistent formatting and aligns with the AI-generated summary.
104-104: LGTM!The assignment of the
valueproperty based on the detected unit measurement ensures consistent formatting and aligns with the AI-generated summary.
109-109: LGTM!The assignment of the
valueproperty based on the detected unit measurement ensures consistent formatting and aligns with the AI-generated summary.
114-114: LGTM!The assignment of the
valueproperty based on the detected unit measurement ensures consistent formatting and aligns with the AI-generated summary.
119-119: LGTM!The changes in the
detect_unit_measurementmethod and the assignment of thevalueproperty based on the detected unit measurement align with the AI-generated summary and improve the robustness of the code.Also applies to: 154-156, 158-158
classes/views/styles/components/FrmStyleComponent.php (1)
136-138: LGTM!The conditional statement to append a custom class name to the default wrapper class is a useful addition. It enhances the flexibility of the component by allowing dynamic class names based on the provided data, which can be beneficial for styling purposes.
The change is implemented correctly and does not introduce any apparent issues or side effects. The existing logic for appending a specific class when
will_changeis set remains intact, ensuring backward compatibility.classes/views/styles/_quick-settings.php (2)
135-135: LGTM!The hidden input field ensures that the base font size is not applied when advanced settings are not enabled. This change aligns with the PR objective of fixing the error fields style issue.
145-145: LGTM!The added
frm-base-font-sizeclass to the slider component provides a way to target and customize the base font size slider specifically. This change aligns with the PR objective of fixing the error fields style issue.classes/views/styles/components/templates/slider.php (6)
19-19: LGTM!The change from
(int)casting toesc_attr()for thevalueattribute of the vertical value input field is a good security enhancement. It ensures proper escaping of the value for HTML output, preventing potential XSS vulnerabilities and handling non-integer values correctly.
41-41: LGTM!The change from
(int)casting toesc_attr()for thevalueattribute of the top value input field is a good security enhancement. It ensures proper escaping of the value for HTML output, preventing potential XSS vulnerabilities and handling non-integer values correctly.
63-63: LGTM!The change from
(int)casting toesc_attr()for thevalueattribute of the bottom value input field is a good security enhancement. It ensures proper escaping of the value for HTML output, preventing potential XSS vulnerabilities and handling non-integer values correctly.
85-85: LGTM!The change from
(int)casting toesc_attr()for thevalueattribute of the horizontal value input field is a good security enhancement. It ensures proper escaping of the value for HTML output, preventing potential XSS vulnerabilities and handling non-integer values correctly.
107-107: LGTM!The change from
(int)casting toesc_attr()for thevalueattribute of the left value input field is a good security enhancement. It ensures proper escaping of the value for HTML output, preventing potential XSS vulnerabilities and handling non-integer values correctly.
129-129: LGTM!The changes from
(int)casting toesc_attr()for thevalueattributes of the right value, field value, and independent field value input fields are good security enhancements. They ensure proper escaping of the values for HTML output, preventing potential XSS vulnerabilities and handling non-integer values correctly.Also applies to: 158-158, 183-183, 210-210
css/admin/style-components.css (2)
377-379: LGTM!The CSS rule is correctly scoped to the disabled and empty state of the slider component, and setting an explicit width for the input field in this state is a good practice for better layout control.
380-382: LGTM!The CSS rule is correctly scoped to the disabled and empty state of the slider component, and setting an explicit width for the select element in this state is a good practice for better layout control.
js/src/admin/styles/slider-style-component.js (4)
133-136: LGTM!The code segment correctly handles the case when the selected unit is an empty string by adding the
frm-disabledandfrm-emptyclasses to the element and preventing further processing.
146-146: LGTM!The code segment correctly removes the
frm-disabledandfrm-emptyclasses from the element when a valid unit is selected, ensuring that the slider is enabled and the empty state is cleared.
288-290: LGTM!The code segment correctly adds a check to exit early from the
initSliderWidthmethod if the slider is disabled. This prevents unnecessary calculations and updates to the slider width when it is in a disabled state, improving efficiency.
456-463: LGTM!The code segment correctly updates a hidden input field when the slider is associated with the "Base Font Size" setting. This change helps avoid conflicts with other font size adjustments in different settings by indicating that the "Base Font Size" has been adjusted.
js/formidable_styles.js (1)
1-1: Suggestion: Review the original, unminified source code.The provided code is heavily minified, making it hard to read and understand. For a more effective code review, it's recommended to review the original, unminified source code instead. Reviewing minified code can miss potential issues and make maintenance harder.
Tools
Biome
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 1-1: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1982 +/- ##
============================================
- Coverage 27.42% 27.40% -0.02%
- Complexity 7960 8002 +42
============================================
Files 125 125
Lines 26568 26693 +125
============================================
+ Hits 7287 7316 +29
- Misses 19281 19377 +96 ☔ View full report in Codecov by Sentry. |
Crabcyborg
requested review from
shervElmi
and removed request for
garretlaxton
There was a problem hiding this comment.
I haven't tested this yet but it looks good to me.
I assigned @shervElmi to review as well to get some additional eyes on this.
Thanks @Liviu-p!
🚀
|
@Liviu-p, I'm working on QA and reviewing the changes as well. Could you provide some details about this PR, please? Thanks. |
Hey @shervElmi, it provides fixes for few reported issues: |
3 participants
No description provided.