We add RTL tools for persian people that use from your platform #37176
Conversation
WalkthroughThis pull request involves the deletion of several GitHub Actions workflow files related to building and testing Storybook applications, as well as the introduction of a new workflow for building the Appsmith application. Additionally, it includes updates to various components and files to enhance typography support, particularly with the addition of new font families "YekanBakh" and "VazirMatn." The changes also introduce a new date picker widget and modifications to existing widgets to support right-to-left text direction. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 28
🧹 Outside diff range and nitpick comments (20)
app/client/packages/dsl/src/migrate/helpers/widget-configs.json (1)
388-395: Fix spacing inconsistency in VazirMatn entryThe additions look good, but there's inconsistent spacing in the JSON properties.
{ - "label" : "VazirMatn", - "value" : "VazirMatn" + "label": "VazirMatn", + "value": "VazirMatn" }app/client/src/widgets/MyDatePickerWidget/component/utils.ts (1)
3-3: Add JSDoc documentation with Persian calendar format examplesSince this is part of RTL support for Persian users, documentation should include Persian calendar format examples.
Add documentation like:
/** * Parses a date string according to the specified format * @param dateStr - The date string to parse (e.g., '1402/06/15' for Persian date) * @param dateFormat - The format string (e.g., 'YYYY/MM/DD') * @returns Parsed Date object or null if invalid */app/client/src/widgets/MyDatePickerWidget/widget/parseDerivedProperties.ts (2)
1-2: Improve type safety of the raw-loader import.The ts-expect-error comment is too broad. Consider creating a proper type declaration for raw-loader or use a more specific error suppression.
-// @ts-expect-error: loader types not available +// @ts-expect-error: Raw loader types are not available in the TypeScript ecosystem
4-8: Convert TODOs to tracked issues.These TODOs should be tracked in the issue system, especially the unit tests which are critical for regex-based parsing.
Would you like me to create GitHub issues for tracking these TODOs?
app/client/src/widgets/MyDatePickerWidget/widget/derived.js (1)
28-29: Remove empty commentThe trailing comment on line 28 serves no purpose and should be removed.
app/client/src/pages/Editor/ThemePropertyPane/controls/ThemeFontControl.tsx (2)
Line range hint
12-16: Add RTL support to font previewThe font preview text "Aa" doesn't indicate RTL support for Persian fonts.
Consider updating the FontText component:
const FontText = styled.div` border-radius: var(--ads-v2-border-radius); border: 1px solid var(--ads-v2-color-border); font-size: 11px; height: 18px; width: 18px; + direction: ${props => props.isRTL ? 'rtl' : 'ltr'}; `;And update the preview text conditionally:
<FontText className="flex items-center justify-center" isRTL={option === 'Rubik'} > {option === 'Rubik' ? 'آا' : 'Aa'} </FontText>
Line range hint
6-12: Consider type-safety for font optionsThe current implementation uses string arrays without type safety for font names.
Consider adding an enum or const object for supported fonts:
export const SUPPORTED_FONTS = { YEKAN_BAKH: 'Rubik', // Internal value SYSTEM_DEFAULT: 'System Default', // Add other fonts } as const; type SupportedFont = typeof SUPPORTED_FONTS[keyof typeof SUPPORTED_FONTS]; interface ThemeFontControlProps { // ... other props options: SupportedFont[]; selectedOption: SupportedFont; }app/client/packages/design-system/theming/src/hooks/src/useTypography.tsx (1)
15-21: Simplify the boolean conditionThe function logic is correct, but the Boolean() call is redundant.
- return !Boolean(fontFamily) || + return !fontFamily || fontFamily == null || fontFamily === "System Default" ? appleSystem : FONT_METRICS[fontFamily];🧰 Tools
🪛 Biome
[error] 16-16: Avoid redundant
BooleancallIt is not necessary to use
Booleancall when a value will already be coerced to a boolean.
Unsafe fix: Remove redundantBooleancall(lint/complexity/noExtraBooleanCast)
.github/workflows/github-commit.yml (1)
1-81: Consider adding RTL-specific validation stepsSince this workflow is part of RTL support implementation, consider adding steps to validate RTL-specific features:
- Visual regression tests for RTL layouts
- Font bundle verification for Persian fonts
- RTL text rendering tests
Would you like assistance in implementing these validation steps?
🧰 Tools
🪛 actionlint
18-18: property "get_version" is not defined in object type {}
(expression)
52-52: property "run_result" is not defined in object type {}
(expression)
62-62: property "run_result" is not defined in object type {}
(expression)
🪛 yamllint
[error] 81-81: no new line character at the end of file
(new-line-at-end-of-file)
app/client/packages/design-system/theming/src/hooks/src/useTheme.tsx (1)
Line range hint
1-131: Consider documenting RTL font family supportSince this change introduces RTL support for Persian users, it would be helpful to add JSDoc comments explaining the font family feature and its intended use case.
+/** + * A hook that provides theme functionality with RTL support. + * @param props - Theme configuration including font family support for RTL languages + * @returns The configured theme object + */ export function useTheme(props: UseThemeProps = {}) {app/client/src/widgets/MyDatePickerWidget/widget/constants.ts (1)
1-149: Add locale configuration support.The date picker should respect the user's locale settings for proper RTL support.
Consider creating a locale configuration system that:
- Allows switching between Gregorian and Persian calendars
- Supports RTL text direction automatically
- Handles number localization (Western vs Persian numerals)
Would you like me to provide a detailed implementation example?
app/client/packages/design-system/theming/src/token/src/TokensAccessor.ts (1)
Font metrics need to be defined for RTL fonts
The VazirMatn and YekanBakh fonts are properly included in the system themes and CSS, but their font metrics are missing from the FONT_METRICS constant. This could affect text rendering and layout calculations.
- Add font metrics for "VazirMatn" and "YekanBakh" in
app/client/packages/design-system/theming/src/token/src/types.ts- Include appropriate line height, font size, and spacing metrics specific to RTL text rendering
🔗 Analysis chain
Line range hint
1-270: Verify RTL font metrics integrationSince this PR aims to add RTL support for Persian users, we should verify that the font family changes are properly integrated with font metrics for RTL text rendering.
Let me check the font-related type definitions and configurations to verify RTL support.
Let me check for font metrics and RTL-specific configurations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for RTL-related font metrics and configurations echo "Checking for RTL font configurations..." rg -l "YekanBakh|VazirMatn|rtl.*font|font.*rtl" # Check for font metric definitions echo "Checking for font metric definitions..." ast-grep --pattern 'const FONT_METRICS = { $$$ }'Length of output: 1036
Script:
#!/bin/bash # Check font family type definitions echo "Checking FontFamily type definition..." rg -A 10 "type FontFamily" app/client/packages/design-system/theming/src/token/src/types.ts # Check font configurations in system themes echo "Checking system theme font configurations..." cat app/server/appsmith-server/src/main/resources/system-themes.json | grep -A 10 "fontFamily" # Check font-related components echo "Checking font control component..." cat app/client/src/pages/Editor/ThemePropertyPane/controls/ThemeFontControl.tsxLength of output: 18770
Script:
#!/bin/bash # Check for font metrics definitions echo "Checking for font metrics..." rg -A 10 "FONT_METRICS" app/client/packages/design-system/theming/src/token/src/ # Check for RTL-specific CSS echo "Checking RTL CSS configurations..." cat app/client/src/assets/fonts/custom/vazir-matn/vazirMatn.css cat app/client/src/assets/fonts/custom/yekanBakh/yekanBakh.cssLength of output: 2537
app/client/src/widgets/index.ts (1)
91-91: Consider renaming the widget to follow naming conventions.The prefix "My" doesn't align with the project's widget naming patterns. Consider a more descriptive name like
RTLDatePickerWidgetorPersianDatePickerWidgetto better reflect its purpose.app/client/src/pages/AppViewer/index.tsx (2)
Line range hint
211-239: Consider making RTL direction configurable based on localeWhile adding RTL support is valuable for Persian users, applying it globally might affect other users. Consider these improvements:
- <div style={{ direction: "rtl" }}> + <div style={{ direction: isRTLLanguage ? "rtl" : "ltr" }}>You'll need to:
- Add a locale selector or configuration
- Create an isRTLLanguage utility to determine text direction
- Consider using CSS logical properties for better RTL/LTR support
RTL implementation needs adjustment to prevent layout conflicts
The RTL wrapper at the root level conflicts with existing directional styles:
- Navigation components use explicit
flex-direction: row/columnwhich may not adapt properly to RTLmargin-leftin SidebarProfileComponent won't flip in RTL context- No RTL-aware theme configurations found to handle bidirectional layouts systematically
Recommendations:
- Use CSS logical properties (e.g.,
margin-inline-startinstead ofmargin-left)- Consider implementing RTL support through a theme provider rather than a div wrapper
- Add RTL-specific styled-components variants for navigation components
🔗 Analysis chain
Line range hint
211-239: Verify RTL compatibility of child componentsThe global RTL wrapper might affect components that should maintain LTR layout (like numbers, code snippets, or third-party components).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for potential RTL-sensitive components rg -l 'direction:|float:|margin-left:|margin-right:|left:|right:' --type tsx --type jsx --type cssLength of output: 128
Script:
#!/bin/bash # Search for potential RTL-sensitive components with correct file type flags rg -l 'direction:|float:|margin-left:|margin-right:|left:|right:' -t typescript -t javascriptreact -t typescriptreact -t css # Specifically look for RTL-related code in the AppViewer directory rg -l 'direction:|float:|margin-left:|margin-right:|left:|right:' app/client/src/pages/AppViewer/ # Search for any RTL-specific utilities or hooks rg -l 'useRTL|rtl-|RTLProps|isRTL' -t typescript -t javascriptreact -t typescriptreact # Look for styled-components with RTL-sensitive properties ast-grep --pattern 'styled.$_` $$$ (left|right|margin-left|margin-right|float|direction) $$$ `'Length of output: 1016
Script:
#!/bin/bash # Let's examine the specific RTL-sensitive styling in these files rg 'direction:|float:|margin-left:|margin-right:|left:|right:' -A 3 app/client/src/pages/AppViewer/Navigation/components/MenuText.styled.tsx app/client/src/pages/AppViewer/Navigation/components/TopHeader.styled.tsx app/client/src/pages/AppViewer/Navigation/components/SidebarProfileComponent.styled.tsx # Check if there are any existing RTL handling mechanisms rg 'direction' -A 3 app/client/src/pages/AppViewer/index.tsx # Look for any theme-related RTL configurations rg 'direction|rtl' app/client/src/constants/ThemeConstants.ts app/client/src/constants/Themes.ts 2>/dev/null # Check for any RTL-related props or interfaces rg 'interface.*Props' -A 5 app/client/src/pages/AppViewer/index.tsxLength of output: 2054
.github/workflows/github-release.yml (1)
258-259: Simplify server artifacts preparation logic.The conditional check for script existence is unnecessary since this is a controlled environment.
-if [[ -f scripts/prepare_server_artifacts.sh ]]; then - scripts/prepare_server_artifacts.sh -fi +scripts/prepare_server_artifacts.shapp/client/src/widgets/TextWidget/component/index.tsx (1)
311-311: Add documentation for RTL supportConsider adding JSDoc documentation to describe the RTL feature and its intended use for Persian language support.
+ /** + * @property {boolean} rtl - Enables right-to-left text direction for Persian language support + */Also applies to: 320-320
app/client/src/widgets/ButtonWidget/widget/index.tsx (1)
407-450: LGTM! Consider tightening font size validation.The font size implementation uses consistent rem units with clear labeling.
Add allowed values validation:
validation: { type: ValidationTypes.TEXT, + params: { + allowedValues: [ + "0.875rem", + "1rem", + "1.25rem", + "1.875rem", + "3rem", + "3.75rem" + ], + default: "1rem" + } },app/client/src/widgets/MyDatePickerWidget/component/index.tsx (1)
339-339: Remove unnecessary console.log statementThe
console.logat line 339 appears to be a debugging statement and should be removed to clean up the code.Apply this diff to remove the statement:
- console.log("test", this.props.selectedDate)app/client/src/widgets/MyDatePickerWidget/widget/index.tsx (1)
454-471: Consider enabling 'onFocus' and 'onBlur' in the property paneThe
onFocusandonBlurevent handlers are implemented and passed to theDatePickerComponent, but their configurations are commented out in the property pane. Uncommenting these properties will allow users to configure actions for these events.Apply this diff to enable them:
- // { - // propertyName: "onFocus", - // label: "onFocus", - // helpText: "when the date picker receives focus", - // controlType: "ACTION_SELECTOR", - // isJSConvertible: true, - // isBindProperty: true, - // isTriggerProperty: true, - // }, - // { - // propertyName: "onBlur", - // label: "onBlur", - // helpText: "when the date picker loses focus", - // controlType: "ACTION_SELECTOR", - // isJSConvertible: true, - // isBindProperty: true, - // isTriggerProperty: true, - // }, + { + propertyName: "onFocus", + label: "onFocus", + helpText: "when the date picker receives focus", + controlType: "ACTION_SELECTOR", + isJSConvertible: true, + isBindProperty: true, + isTriggerProperty: true, + }, + { + propertyName: "onBlur", + label: "onBlur", + helpText: "when the date picker loses focus", + controlType: "ACTION_SELECTOR", + isJSConvertible: true, + isBindProperty: true, + isTriggerProperty: true, + },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (4)
app/client/src/assets/fonts/custom/vazir-matn/vazir-matn-01/Vazirmatn.ttfis excluded by!**/*.ttfapp/client/src/assets/fonts/custom/yekanBakh/yekanBakh/YekanBakhFaNum-VF.ttfis excluded by!**/*.ttfapp/client/src/widgets/MyDatePickerWidget/icon.svgis excluded by!**/*.svgapp/client/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (33)
.github/workflows/build-chromatic.yml(0 hunks).github/workflows/build-storybook.yml(0 hunks).github/workflows/ci-test-hosted.yml(0 hunks).github/workflows/github-commit.yml(1 hunks).github/workflows/github-release.yml(5 hunks).github/workflows/sync-release-to-pg.yml(0 hunks)app/client/package.json(2 hunks)app/client/packages/design-system/theming/src/hooks/src/useTheme.tsx(7 hunks)app/client/packages/design-system/theming/src/hooks/src/useTypography.tsx(3 hunks)app/client/packages/design-system/theming/src/token/src/TokensAccessor.ts(5 hunks)app/client/packages/design-system/theming/src/token/src/types.ts(3 hunks)app/client/packages/dsl/src/migrate/helpers/widget-configs.json(1 hunks)app/client/src/assets/fonts/custom/index.css(1 hunks)app/client/src/assets/fonts/custom/vazir-matn/vazirMatn.css(1 hunks)app/client/src/assets/fonts/custom/yekanBakh/yekanBakh.css(1 hunks)app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/unitTestUtils.ts(13 hunks)app/client/src/constants/WidgetConstants.tsx(1 hunks)app/client/src/pages/AppViewer/Navigation/index.tsx(1 hunks)app/client/src/pages/AppViewer/index.tsx(2 hunks)app/client/src/pages/Editor/ThemePropertyPane/controls/ThemeFontControl.tsx(1 hunks)app/client/src/widgets/ButtonWidget/widget/index.tsx(1 hunks)app/client/src/widgets/MyDatePickerWidget/component/index.tsx(1 hunks)app/client/src/widgets/MyDatePickerWidget/component/utils.ts(1 hunks)app/client/src/widgets/MyDatePickerWidget/constants.ts(1 hunks)app/client/src/widgets/MyDatePickerWidget/index.ts(1 hunks)app/client/src/widgets/MyDatePickerWidget/widget/constants.ts(1 hunks)app/client/src/widgets/MyDatePickerWidget/widget/derived.js(1 hunks)app/client/src/widgets/MyDatePickerWidget/widget/index.tsx(1 hunks)app/client/src/widgets/MyDatePickerWidget/widget/parseDerivedProperties.ts(1 hunks)app/client/src/widgets/TextWidget/component/index.tsx(5 hunks)app/client/src/widgets/TextWidget/widget/index.tsx(4 hunks)app/client/src/widgets/index.ts(2 hunks)app/server/appsmith-server/src/main/resources/system-themes.json(16 hunks)
🔥 Files not summarized due to errors (1)
- app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/unitTestUtils.ts: Error: Server error: no LLM provider could handle the message
💤 Files with no reviewable changes (4)
- .github/workflows/build-chromatic.yml
- .github/workflows/build-storybook.yml
- .github/workflows/ci-test-hosted.yml
- .github/workflows/sync-release-to-pg.yml
✅ Files skipped from review due to trivial changes (4)
- app/client/src/assets/fonts/custom/index.css
- app/client/src/assets/fonts/custom/vazir-matn/vazirMatn.css
- app/client/src/assets/fonts/custom/yekanBakh/yekanBakh.css
- app/client/src/widgets/MyDatePickerWidget/index.ts
🧰 Additional context used
🪛 actionlint
.github/workflows/github-commit.yml
18-18: property "get_version" is not defined in object type {}
(expression)
52-52: property "run_result" is not defined in object type {}
(expression)
62-62: property "run_result" is not defined in object type {}
(expression)
🪛 yamllint
.github/workflows/github-commit.yml
[error] 81-81: no new line character at the end of file
(new-line-at-end-of-file)
🪛 Biome
app/client/packages/design-system/theming/src/hooks/src/useTypography.tsx
[error] 16-16: Avoid redundant Boolean call
It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call
(lint/complexity/noExtraBooleanCast)
app/client/src/widgets/MyDatePickerWidget/component/index.tsx
[error] 331-331: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 335-335: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
app/client/src/widgets/MyDatePickerWidget/widget/derived.js
[error] 12-12: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
[error] 15-15: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
[error] 16-16: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
[error] 19-19: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
[error] 20-20: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
app/client/src/widgets/MyDatePickerWidget/widget/parseDerivedProperties.ts
[error] 15-15: 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 (40)
app/client/packages/dsl/src/migrate/helpers/widget-configs.json (1)
388-395: Verify font file availability
The font family additions are appropriate for RTL support. Let's verify that the corresponding font files are included in the project assets.
✅ Verification successful
Font files are properly included and configured ✅
Both YekanBakh and VazirMatn font files are present in the project assets with their corresponding CSS configurations:
- YekanBakh at
app/client/src/assets/fonts/custom/yekanBakh/yekanBakh/YekanBakhFaNum-VF.ttf - VazirMatn at
app/client/src/assets/fonts/custom/vazir-matn/vazir-matn-01/Vazirmatn.ttf
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for YekanBakh and VazirMatn font files in the project
echo "Searching for font files..."
fd -e ttf -e woff -e woff2 -e otf "YekanBakh|VazirMatn"
Length of output: 199
Script:
#!/bin/bash
# Let's also check for VazirMatn font files in a case-insensitive manner
echo "Searching for VazirMatn font files..."
fd -i "vazir" -e ttf -e woff -e woff2 -e otf
# Check font-family declarations in CSS/SCSS files
echo -e "\nChecking font-family declarations in stylesheets..."
rg -i "font-family.*(?:YekanBakh|Vazir)" --type css -A 2
Length of output: 920
app/client/src/widgets/MyDatePickerWidget/constants.ts (1)
1-1: LGTM! Clear and well-defined type alias.
The DatePickerType union type provides a clear contract for the widget's type options.
app/client/src/widgets/MyDatePickerWidget/widget/parseDerivedProperties.ts (2)
36-36: LGTM!
The export statement is clean and follows standard practices.
9-11: Consider using an AST parser instead of regex.
The current regex pattern is complex and potentially fragile. A proper AST parser (like @babel/parser) would be more reliable for parsing JavaScript code.
Let's check if there are any edge cases in the codebase that might break this regex:
app/client/src/widgets/MyDatePickerWidget/widget/derived.js (1)
1-29: Consider timezone handling for Persian dates
Since this is part of RTL tools for Persian users, the date validation should account for timezone differences and Persian calendar specifics.
Let me verify if there are any timezone-related utilities in the codebase:
Would you like assistance implementing proper Persian calendar support?
🧰 Tools
🪛 Biome
[error] 12-12: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
[error] 15-15: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
[error] 16-16: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
[error] 19-19: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
[error] 20-20: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
app/client/packages/design-system/theming/src/hooks/src/useTypography.tsx (3)
7-12: LGTM: Type imports are properly structured
The addition of FontFamily type and FONT_METRICS import provides proper typing support for the new font families.
65-72: LGTM: Hook implementation is correct
The fontFamily parameter is properly integrated into the hook and its dependencies.
Line range hint 22-41: Verify font metrics for Persian fonts
The fontMetrics integration looks correct, but we should verify the metrics for new Persian fonts.
✅ Verification successful
Let me search for font-related code and metrics definitions with proper file extensions and a broader search pattern to understand the font metrics implementation.
Let me check the font metrics implementation and Persian font integration details.
Font metrics for Persian fonts are properly integrated
The VazirMatn font metrics are correctly imported from @capsizecss/metrics/vazirmatn and integrated into the FONT_METRICS constant. The font metrics integration in useTypography.tsx properly handles these metrics through the getFontMetrics function.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if YekanBakh and VazirMatn are properly defined in FONT_METRICS
rg -A 5 "YekanBakh|VazirMatn" --type typescript
Length of output: 84
Script:
#!/bin/bash
# Search for font metrics definitions and related code
rg -g "*.{ts,tsx,js,jsx}" "YekanBakh|VazirMatn|FONT_METRICS|getFontMetrics" -A 5
# Also search for any font-related configuration files
fd -e ts -e tsx -e js -e jsx -x grep -l "font"
Length of output: 54840
Script:
#!/bin/bash
# Let's check the font metrics implementation specifically
ast-grep --pattern 'const FONT_METRICS = {
$$$
}'
# Also search for VazirMatn metrics import and usage
rg -g "*.{ts,tsx}" "import.*VazirMatn.*from.*@capsizecss/metrics" -A 5
Length of output: 656
🧰 Tools
🪛 Biome
[error] 16-16: Avoid redundant Boolean call
It is not necessary to use Boolean call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean call
(lint/complexity/noExtraBooleanCast)
app/client/packages/design-system/theming/src/hooks/src/useTheme.tsx (3)
13-13: LGTM: Type definitions are properly structured
The FontFamily type import and interface extension are correctly implemented to support the new font family feature.
Also applies to: 25-25
115-115: LGTM: Theme return value and dependencies are properly updated
The fontFamily is correctly exposed in the theme object and added to the dependency array.
Also applies to: 127-127
30-30: Verify typography hook integration
The integration looks correct, but we should verify that the useTypography hook properly handles undefined fontFamily values for backward compatibility.
Also applies to: 46-49
app/client/packages/design-system/theming/src/token/src/types.ts (4)
39-39: LGTM: Clean interface extension
The optional fontFamily property is a good approach for maintaining backward compatibility while adding RTL support.
122-122: LGTM: Well-defined type
The FontFamily type definition elegantly combines system and custom fonts while maintaining type safety.
83-98:
Add YekanBakh to FONT_METRICS
After importing YekanBakh metrics, add it to the FONT_METRICS constant:
export const FONT_METRICS = {
Poppins: poppins,
// ... other fonts
VazirMatn: VazirMatn,
+ YekanBakh: YekanBakh,
} as const;Likely invalid or redundant comment.
Line range hint 1-150: Verify complete RTL implementation
Let's ensure all necessary RTL-related properties are implemented across the codebase.
✅ Verification successful
RTL support is properly implemented with feature flag control
The codebase has comprehensive RTL support implementation:
- RTL feature is controlled by
license_widget_rtl_support_enabledfeature flag - RTL support is implemented in key widgets:
- Select Widget
- MultiSelect Widget
- Input Widget
- Each implementation includes:
- RTL direction CSS properties
- RTL-specific class names and attributes
- Toggle controls in property pane
- Proper test coverage with both enabled/disabled states
- Correct fallback to LTR when disabled
The implementation follows best practices with proper feature flagging, styling, and testing. No issues found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for RTL-related properties and configurations
echo "Checking for RTL implementations..."
# Search for direction-related CSS properties
rg -l "direction:\s*(rtl|ltr)" --type css --type typescript --type javascript
# Search for RTL-specific components or utilities
rg -l "(useRTL|RTLProvider|rtl-|direction:)" --type typescript --type javascript
# Search for text-align properties
rg -l "text-align:\s*(right|left|start|end)" --type css --type typescript --type javascript
Length of output: 442
Script:
#!/bin/bash
# Let's try a different approach to check RTL implementations
# Search for RTL-related patterns without file type restrictions
echo "Checking for RTL implementations..."
# Search for direction properties and RTL-related code
rg -l "direction[\"'\s:]"
# Search for RTL-specific components, utilities, or configurations
rg -l "(useRTL|RTLProvider|rtl-|isRTL|RTL)"
# Search for text alignment patterns that might indicate RTL support
rg -l "text-align[\"'\s:]"
# Look specifically in theme-related files
fd "theme|rtl" -e ts -e js -e tsx -e jsx --exec cat {}
Length of output: 331359
Script:
#!/bin/bash
# Let's check for RTL-specific implementations in the codebase
echo "Checking for RTL implementations..."
# Search for RTL-related imports and configurations
rg -l "rtl|RTL|direction|text-align|right-to-left" --type ts --type tsx
# Search for RTL-specific style properties and classes
rg -p "\.rtl|direction:|text-align:|right-to-left" --type ts --type tsx
# Search for RTL-related tests
rg -l "RTL" "app/client/cypress/e2e"
Length of output: 564
Script:
#!/bin/bash
# Let's try a different approach to check RTL implementations
echo "Checking for RTL implementations..."
# Search for RTL-related files and content
fd -e ts -e tsx -x rg -l "rtl|RTL|direction|text-align|right-to-left" {}
# Look at specific test files to understand RTL implementation
cat "app/client/cypress/e2e/Regression/ClientSide/Widgets/Select/RTL_support.ts"
cat "app/client/cypress/e2e/Regression/ClientSide/Widgets/Multiselect/MultiSelectRTL_support.ts"
cat "app/client/cypress/e2e/Regression/ClientSide/Widgets/Input/InputRTL_support.ts"
# Check for RTL-related feature flags or configurations
rg -p "widget_rtl_support"
Length of output: 33366
app/client/src/pages/AppViewer/Navigation/index.tsx (1)
Line range hint 122-139: Verify RTL compatibility with child components.
The current implementation sets RTL at the container level. Ensure all child components (especially TopHeader, PageMenu) properly handle RTL layout.
app/client/packages/design-system/theming/src/token/src/TokensAccessor.ts (5)
11-11: LGTM: FontFamily type import added correctly
The FontFamily type import is properly added to support the new font family functionality.
28-28: LGTM: Private fontFamily property follows class conventions
The optional fontFamily property is correctly declared as private, maintaining consistency with other token properties in the class.
44-44: LGTM: Constructor properly initialized with fontFamily
The fontFamily parameter is correctly added to the constructor parameters and properly assigned to the instance property.
Also applies to: 59-59
144-144: LGTM: Getter methods properly implemented
The getFontFamily getter and its integration into getAllTokens follow the established patterns in the class.
Also applies to: 148-151
66-71: 🛠️ Refactor suggestion
Consider adding validation in updateFontFamily method
While the method implementation is correct, consider adding validation to ensure the fontFamily value meets expected format/requirements for RTL support.
Consider adding validation:
updateFontFamily = (fontFamily?: FontFamily) => {
+ if (fontFamily && !this.isValidFontFamily(fontFamily)) {
+ throw new Error('Invalid font family format');
+ }
this.fontFamily = fontFamily;
};app/client/src/widgets/index.ts (1)
91-91: Verify RTL support implementation.
Let's verify the RTL implementation across the codebase to ensure consistent support.
Also applies to: 144-144
app/client/src/constants/WidgetConstants.tsx (1)
244-244: Verify RTL support in the widget implementation.
While this constant addition is related to the new date picker widget, ensure that the actual widget implementation properly supports RTL for Persian users as per the PR objectives.
✅ Verification successful
RTL support is properly implemented in the MyDatePicker widget ✅
The widget implementation includes:
- RTL text direction setting via
direction: "rtl" - Persian calendar integration using
react-date-object/calendars/persian - Persian locale support with
react-date-object/locales/persian_fa
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for RTL-related configurations in the date picker widget implementation
rg -l "rtl|direction.*rtl|right-to-left" --type ts --type tsx
Length of output: 91
Script:
#!/bin/bash
# Search for RTL-related configurations in the date picker widget implementation
# Using correct file extensions and including both rtl and direction patterns
rg -l "rtl|direction.*rtl|right-to-left" --type ts
# Search for the MyDatePicker widget implementation
fd "MyDatePicker" -e ts -e tsx
# Search for any direction or text alignment related code in potential widget files
rg -i "direction|align|rtl" -g "*DatePicker*" -g "*date*picker*"
Length of output: 2884
Script:
#!/bin/bash
# Let's check the actual implementation of MyDatePickerWidget for RTL support
rg -A 10 "rtl|direction.*rtl|right-to-left" app/client/src/widgets/MyDatePickerWidget/component/index.tsx
# Also check for any Persian/Farsi calendar or locale related configurations
rg -i "persian|farsi|fa[-_]IR" app/client/src/widgets/MyDatePickerWidget/component/index.tsx
# Check if there are any style-related files for the MyDatePicker
fd "style|styled" -e ts -e tsx app/client/src/widgets/MyDatePickerWidget/
Length of output: 1132
.github/workflows/github-release.yml (3)
217-218: LGTM: Docker Buildx setup.
Proper setup for multi-architecture builds using Docker Buildx.
25-34: Verify Docker Hub credentials rotation process.
The workflow now uses new Docker Hub credentials (DOCKER_HUB_USERNAME2, DOCKER_HUB_ACCESS_TOKEN2). Ensure the old credentials are properly deprecated after this change is merged.
274-278: Verify base image compatibility with RTL requirements.
Since this PR adds RTL support for Persian users, ensure the base image appsmith/base-ce:nightly includes all necessary fonts and RTL-specific dependencies.
✅ Verification successful
Base image is compatible with RTL requirements
The codebase already has extensive RTL support implemented across multiple components including text widgets, select widgets, and input components. The base image appsmith/base-ce:nightly is used as a foundation, and RTL functionality is handled at the application level through React components and CSS styling, not through system-level font dependencies.
- RTL support is implemented in:
- Text and Input widgets
- Select and MultiSelect components
- Rich Text Editor
- Table widget
- Date Picker
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check Dockerfile for font-related configurations
rg -A 5 'font|rtl|persian|arabic' Dockerfile
Length of output: 140
Script:
#!/bin/bash
# Search for Dockerfile in the repository
fd Dockerfile
# Check base image definition and dependencies in deployment files
rg -A 5 "FROM.*base-ce:nightly" --type yaml
rg -A 5 "FROM.*base-ce:nightly" --type dockerfile
# Look for font-related configurations in deployment files
rg -A 5 'font|rtl|persian|arabic' --type yaml
rg -A 5 'font|rtl|persian|arabic' --type dockerfile
# Check for any RTL-related configurations in the codebase
rg -A 5 'direction:\s*rtl|dir="rtl"|dir=rtl' --type ts --type tsx --type js --type jsx
Length of output: 505
Script:
#!/bin/bash
# Check base image and font configurations in all Dockerfiles
cat Dockerfile
cat deploy/heroku/Dockerfile
cat deploy/docker/route-tests/Dockerfile
cat app/client/cypress/Dockerfile
# Check RTL configurations in the client app
rg -A 5 'direction.*rtl|dir.*rtl' app/client/src
Length of output: 13519
app/client/src/widgets/TextWidget/component/index.tsx (3)
113-113: LGTM: Interface changes for RTL support are well-defined
The optional boolean rtl property is correctly added to both interfaces, maintaining consistency and type safety.
Also applies to: 229-229
157-157: 🛠️ Refactor suggestion
Consider adjusting text-align for RTL mode
While the direction property is correctly implemented, RTL text typically needs right alignment. Consider updating the text-align logic to account for RTL mode.
- text-align: ${(props) => props.textAlign.toLowerCase()};
+ text-align: ${(props) => props.rtl ? "right" : props.textAlign.toLowerCase()};
direction: ${(props) => props.rtl ? "rtl" : "ltr"};Likely invalid or redundant comment.
Line range hint 311-320: Verify RTL implementation completeness
Let's verify that RTL support is consistently implemented across related components.
✅ Verification successful
Let me verify the RTL implementation details in the components to ensure completeness.
RTL implementation is properly handled across components
The RTL implementation is consistent across text-based components with proper prop passing and styling:
- TextWidget correctly implements RTL with direction styling
- BaseInputWidget handles RTL in both class names and dir attribute
- Parent widgets properly pass the RTL prop to their children
- RichTextEditor includes RTL-specific styling rules
The implementation follows a consistent pattern where RTL is handled at both the component and styling levels, ensuring proper text direction support.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for RTL implementation consistency across widget components
# Check for rtl prop usage in other components
echo "Checking for RTL prop usage across components..."
rg -l "rtl.*?:" app/client/src/widgets
# Check for potential missing RTL implementations in other text-based widgets
echo "Checking other text-based widgets for RTL support..."
fd -e tsx -e ts -p ".*Widget" app/client/src/widgets | xargs rg -l "text-align|direction"
Length of output: 4906
Script:
#!/bin/bash
# Check the actual RTL implementation in components that use it
echo "Checking RTL implementation in components..."
rg -A 5 "rtl.*?:" app/client/src/widgets/TextWidget/component/index.tsx app/client/src/widgets/BaseInputWidget/component/index.tsx app/client/src/widgets/RichTextEditorWidget/component/index.tsx
# Check if RTL is properly passed from parent widgets
echo "Checking RTL prop passing from parent widgets..."
rg -A 3 "rtl.*?=" app/client/src/widgets/TextWidget/widget/index.tsx app/client/src/widgets/InputWidgetV2/widget/index.tsx app/client/src/widgets/RichTextEditorWidget/widget/index.tsx
Length of output: 5731
app/client/package.json (2)
188-188: LGTM: react-multi-date-picker is a solid choice for Persian calendar.
The addition of react-multi-date-picker is appropriate as it provides built-in support for Persian calendar and RTL layout.
129-129: Verify dayjs locale configuration for Persian support.
The dayjs update to 1.11.10 is appropriate for RTL/Persian support. However, ensure that Persian locale is properly configured in the application.
app/client/src/widgets/TextWidget/widget/index.tsx (3)
241-250: LGTM: RTL configuration is well-implemented.
The RTL property configuration follows the established pattern with appropriate validation and help text.
592-592: Verify RTL implementation in TextComponent.
The RTL prop is correctly passed to TextComponent, but we should verify its implementation.
Also applies to: 636-636
315-322: Consider adding font-display property for new fonts.
While adding Persian fonts is good, ensure they're loaded efficiently to prevent FOUT (Flash of Unstyled Text).
✅ Verification successful
Font-display property is already properly configured
Both YekanBakh and VazirMatn fonts are correctly configured with font-display: swap, which is the recommended setting for optimal font loading performance.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if font-display is specified for the new fonts
rg -g '*.css' -g '*.scss' "font-family.*(?:YekanBakh|VazirMatn)"
Length of output: 84
Script:
#!/bin/bash
# Search for font-face declarations for the new fonts
rg -g '*.css' -g '*.scss' "@font-face.*(?:YekanBakh|VazirMatn)" -A 5
# Search for any font loading or font configuration
rg -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx' "(?:YekanBakh|VazirMatn).*font"
# Look for font files in the repository
fd -e ttf -e woff -e woff2 -e eot "(?:YekanBakh|VazirMatn)"
# Check for any font-related configuration files
fd "font" -e config -e json
Length of output: 358
Script:
#!/bin/bash
# Check the content of fontData.json
cat app/client/cypress/fixtures/fontData.json
# Look for font-face declarations in any CSS files in the assets/fonts directory
rg -g '*.css' "@font-face" app/client/src/assets/fonts/
# Check if there are any CSS files in the yekanBakh directory
fd -e css app/client/src/assets/fonts/custom/yekanBakh
Length of output: 17725
Script:
#!/bin/bash
# Check the content of the YekanBakh and VazirMatn font CSS files
cat app/client/src/assets/fonts/custom/yekanBakh/yekanBakh.css
cat app/client/src/assets/fonts/custom/vazir-matn/vazirMatn.css
Length of output: 385
app/client/src/widgets/ButtonWidget/widget/index.tsx (2)
341-452: LGTM! Font section structure follows widget configuration patterns.
The new Font section is well-organized and properly integrated into the property pane configuration.
344-405: 🛠️ Refactor suggestion
Consider enhancing font family validation and RTL support.
- Add allowed values validation to prevent invalid font selections.
- Consider adding RTL configuration for Persian fonts.
validation: {
type: ValidationTypes.TEXT,
+ params: {
+ allowedValues: [
+ "System Default",
+ "Nunito Sans",
+ "Poppins",
+ "Inter",
+ "Montserrat",
+ "Noto Sans",
+ "Open Sans",
+ "Roboto",
+ "Rubik",
+ "Ubuntu",
+ "YekanBakh",
+ "VazirMatn"
+ ],
+ default: "System Default"
+ }
},Let's verify if RTL configuration exists for these fonts:
app/server/appsmith-server/src/main/resources/system-themes.json (2)
415-415: Ensure consistent font application across themes
The font changes have been consistently applied across all themes, which is good for maintaining a unified RTL experience.
Also applies to: 828-828, 1240-1240, 1648-1648, 2462-2462, 2870-2870, 3278-3278, 3696-3696
37-39: Verify font availability and loading strategy
The addition of Persian fonts needs to be accompanied by proper font loading strategy to ensure availability across different platforms.
✅ Verification successful
Font loading strategy is properly implemented
Both Persian fonts are properly configured with appropriate font loading strategies:
- YekanBakh font is included at
app/client/src/assets/fonts/custom/yekanBakh/yekanBakh/YekanBakhFaNum-VF.ttfwith proper@font-facedeclaration usingfont-display: swap - VazirMatn font is included at
app/client/src/assets/fonts/custom/vazir-matn/vazir-matn-01/Vazirmatn.ttfwith proper@font-facedeclaration usingfont-display: swap
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if font files are included in the assets
fd -e ttf -e woff -e woff2 "VazirMatn|YekanBakh"
# Check if there are any font-face declarations
rg -l "@font-face.*VazirMatn|@font-face.*YekanBakh"
Length of output: 181
Script:
#!/bin/bash
# Check for VazirMatn font files and font-face declarations
fd -e ttf -e woff -e woff2 "Vazir"
rg -l "@font-face.*Vazir"
# Check font loading implementation in CSS/SCSS files
rg -l "font-family.*Vazir|font-family.*YekanBakh" --type css --type scss
# Check for any font loading utilities or configurations
rg -l "fontLoader|loadFonts|fontConfig" --type ts --type js
Length of output: 302
Script:
#!/bin/bash
# Check for font-face declarations in all file types
rg "@font-face" -A 5 -B 1
# Check for font loading related code in client
rg "font-family|@font-face" app/client/src/assets/styles -A 2
# Check for any font configuration in theme files
rg "fontFamily|font-family" app/client/src/constants/ThemeConstants.ts
Length of output: 65111
app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/unitTestUtils.ts (2)
12763-12763: LGTM: Feature flag for RTL support
Clean implementation using feature flag for controlled rollout of RTL widget support.
17162-17162: Verify default font configuration impact
Setting "YekanBakh" as the default appFont might affect the user experience for non-Persian users. Consider making this conditional based on the user's language/locale preference.
Let's verify if there are any locale-based font configurations:
Also applies to: 17598-17598
app/client/src/widgets/MyDatePickerWidget/widget/parseDerivedProperties.ts
Show resolved
Hide resolved
...ient/src/components/editorComponents/PartialImportExport/PartialExportModal/unitTestUtils.ts
Show resolved
Hide resolved
|
@hajrezvan Could you please update your code from release branch? I see quite a lot of old changes unrelated to the essence of your PR. |
|
Thank you |
Description
Tip
Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team).
Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR.
Fixes #
Issue Numberor
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags=""
🔍 Cypress test results
Caution
If you modify the content in this section, you are likely to disrupt the CI result for your PR.
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Release Notes
New Features
MyDatePickerWidgetfor enhanced date selection.ButtonWidgetandTextWidgetto include new font options and support for right-to-left text direction.Bug Fixes
Chores
These changes enhance user experience with improved typography options and a new date picker widget.