feat(web): lang-direction #1808
Conversation
WalkthroughThis pull request introduces modifications to enhance text direction handling across multiple components in the web application. The primary change involves adding the Changes
Suggested labels
Suggested reviewers
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
🪧 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: 1
🧹 Nitpick comments (8)
web/src/pages/Cases/CaseDetails/Voting/VotingHistory.tsx (1)
50-50: Consider adding explicit styles to ReactMarkdownWrapperThe wrapper component is currently empty. Consider adding styles that are common across other markdown wrappers in the codebase for consistency.
web/src/components/EvidenceCard.tsx (1)
61-61: Consider adding explicit styles to ReactMarkdownWrapperThe wrapper component is currently empty. Consider adding styles that are common across other markdown wrappers in the codebase for consistency.
web/src/pages/Cases/CaseDetails/Voting/Classic/JustificationArea.tsx (1)
26-26: LGTM! Consider text alignment propertiesThe addition of
dir="auto"is appropriate for the justification textarea. Since this is a long-form text input area, the automatic direction will help with mixed-language content.Consider adding
text-align: startto ensure consistent alignment behavior across different text directions:const StyledTextarea = styled(Textarea)` width: 100%; height: auto; textarea { height: 200px; border-color: ${({ theme }) => theme.stroke}; + text-align: start; } // ... `;web/src/components/DisputePreview/Alias.tsx (1)
49-49: Consider adding dir="auto" to TextContainer as well.While adding
dir="auto"toAliasContaineris good, consider also adding it toTextContainerfor consistent handling of mixed-language content in the name labels.const TextContainer = styled.div` display: flex; flex-wrap: wrap; max-width: 100%; + dir: auto; > label { color: ${({ theme }) => theme.primaryText}; font-size: 14px; word-wrap: break-word; max-width: 100%; } `;web/src/pages/Resolver/Briefing/Description.tsx (1)
Line range hint
40-44: Consider extracting the placeholder text to a constant.The placeholder text is quite long and would be better maintained as a constant, especially if it needs to be translated or modified in the future.
+const DESCRIPTION_PLACEHOLDER = "eg. Bob hired Alice to develop a website for him. Bob claims the contract was not fully respected, and the website was delivered incomplete. For that reason, he wants to pay part of the agreed payment: 150 DAI. On the other hand, Alice claims she should receive the full payment: 250 DAI."; const Description: React.FC = () => { // ... return ( <Container> <Header text="Describe the case" /> <StyledTextArea dir="auto" onChange={handleWrite} value={disputeData.description} - placeholder="eg. Bob hired Alice to develop a website for him. Bob claims the contract was not fully respected, and the website was delivered incomplete. For that reason, he wants to pay part of the agreed payment: 150 DAI. On the other hand, Alice claims she should receive the full payment: 250 DAI." + placeholder={DESCRIPTION_PLACEHOLDER} /> // ... </Container> ); };web/src/pages/Resolver/Parameters/Category.tsx (1)
51-51: LGTM! Consider internationalizing the placeholder and message.The addition of
dir="auto"is appropriate for handling multilingual category input. As a future enhancement, consider internationalizing the placeholder and message text to fully support multilingual users.web/src/pages/Cases/CaseDetails/Evidence/EvidenceSearch.tsx (1)
51-51: LGTM! Consider internationalizing the placeholder text.The addition of
dir="auto"appropriately handles multilingual search input. As a future enhancement, consider internationalizing the placeholder text to better support multilingual users.web/src/components/DisputeView/DisputeListView.tsx (1)
12-12: LGTM! Consider extracting TruncatedTitle componentThe changes look good:
- Added
dir="auto"for proper language direction handling- Improved layout with
flex: 1for the title- Restored
responsiveSizeimportSince
TruncatedTitleis duplicated in bothDisputeCardViewandDisputeListView, consider extracting it into a shared component.Also applies to: 40-40, 45-45
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
web/src/components/CasesDisplay/Search.tsx(1 hunks)web/src/components/DisputePreview/Alias.tsx(1 hunks)web/src/components/DisputePreview/DisputeContext.tsx(3 hunks)web/src/components/DisputeView/DisputeCardView.tsx(2 hunks)web/src/components/DisputeView/DisputeListView.tsx(2 hunks)web/src/components/EvidenceCard.tsx(3 hunks)web/src/components/Field.tsx(2 hunks)web/src/components/LabeledInput.tsx(1 hunks)web/src/components/Verdict/Answer.tsx(1 hunks)web/src/components/Verdict/FinalDecision.tsx(4 hunks)web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/FormContact.tsx(2 hunks)web/src/pages/Cases/CaseDetails/Evidence/EvidenceSearch.tsx(1 hunks)web/src/pages/Cases/CaseDetails/Evidence/SubmitEvidenceModal.tsx(2 hunks)web/src/pages/Cases/CaseDetails/Voting/Classic/JustificationArea.tsx(1 hunks)web/src/pages/Cases/CaseDetails/Voting/Classic/OptionsContainer.tsx(1 hunks)web/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsx(2 hunks)web/src/pages/Cases/CaseDetails/Voting/VotesDetails/index.tsx(3 hunks)web/src/pages/Cases/CaseDetails/Voting/VotingHistory.tsx(2 hunks)web/src/pages/Resolver/Briefing/Description.tsx(1 hunks)web/src/pages/Resolver/Briefing/Title.tsx(1 hunks)web/src/pages/Resolver/Parameters/Category.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- web/src/pages/Cases/CaseDetails/Voting/Classic/OptionsContainer.tsx
- web/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsx
🧰 Additional context used
📓 Learnings (2)
web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/FormContact.tsx (1)
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1725
File: web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/FormContact.tsx:5-5
Timestamp: 2024-11-12T04:49:37.993Z
Learning: In the codebase, the `isEmpty` function is always called after ensuring the input is defined using `isUndefined`, so `isEmpty` does not need to handle undefined inputs.
web/src/pages/Cases/CaseDetails/Evidence/SubmitEvidenceModal.tsx (2)
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1687
File: web/src/pages/Cases/CaseDetails/Evidence/SubmitEvidenceModal.tsx:67-84
Timestamp: 2024-11-12T04:49:48.061Z
Learning: In the `submitEvidence` function of the SubmitEvidenceModal component, the try-catch block is specifically designed to handle errors from the `constructEvidence` function, which may throw an Error if file upload fails. This ensures that the file is successfully uploaded before proceeding with evidence submission and transaction signing.
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1724
File: web/src/pages/Cases/CaseDetails/Evidence/SubmitEvidenceModal.tsx:86-89
Timestamp: 2024-11-12T04:49:37.993Z
Learning: In the `submitEvidence` function within `SubmitEvidenceModal.tsx`, most errors are caught by `wrapWithToast`, and the `catch` block is mainly for edge cases.
🔇 Additional comments (20)
web/src/pages/Cases/CaseDetails/Voting/VotingHistory.tsx (1)
98-100: LGTM! Proper implementation of text direction handling
The dir="auto" attribute is correctly implemented to handle text direction automatically based on content.
web/src/components/EvidenceCard.tsx (2)
18-21: LGTM! Improved import organization
The imports are now properly organized and grouped by type.
220-220: LGTM! Comprehensive text direction handling
The dir="auto" attribute is correctly implemented at both the container (TopContent) and content (ReactMarkdownWrapper) levels, ensuring proper text direction handling for all content types.
Also applies to: 226-228
web/src/components/Verdict/Answer.tsx (1)
25-25: LGTM! Verify container inheritance
The addition of dir="auto" to the AnswerTitleAndDescription component is appropriate for handling mixed-language content in both the title and description.
Let's verify the parent container's direction properties don't conflict:
✅ Verification successful
Direction property is consistently applied across components
The dir="auto" attribute on AnswerTitleAndDescription aligns with the existing pattern in the codebase:
- Parent components in DisputeContext.tsx already use
dir="auto"for text content - The styled component definitions don't contain any conflicting direction properties
- The component uses simple block/inline display properties that won't interfere with text direction
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for parent styled-components that might affect text direction
rg -t tsx -t jsx "styled.*AnswerTitleAndDescription|AnswerTitle|AnswerDescription" web/src/
Length of output: 121
Script:
#!/bin/bash
# Search for styled components related to Answer components with proper file extension
rg "styled.*AnswerTitleAndDescription|AnswerTitle|AnswerDescription" web/src/components/Verdict/
Length of output: 647
Script:
#!/bin/bash
# Look for the styled component definition and its properties
rg -A 5 "const AnswerTitleAndDescription" web/src/components/Verdict/
Length of output: 71
Script:
#!/bin/bash
# Check for any direction-related styles in the component file
rg "direction|dir=" web/src/components/Verdict/Answer.tsx
Length of output: 129
Script:
#!/bin/bash
# Look for the styled component definitions in the DisputeContext file
rg -A 5 "const AnswerTitleAndDescription|const AnswerTitle|const AnswerDescription" web/src/components/DisputePreview/DisputeContext.tsx
Length of output: 415
Script:
#!/bin/bash
# Check for any direction-related styles in the DisputeContext file
rg "direction|dir=" web/src/components/DisputePreview/DisputeContext.tsx
Length of output: 316
web/src/pages/Resolver/Briefing/Title.tsx (1)
47-47: LGTM! Consider adding RTL testing.
The addition of dir="auto" is appropriate for handling mixed-language title inputs. This change aligns well with the PR's objective of improving text direction support.
To ensure comprehensive RTL support, consider testing with the following languages:
✅ Verification successful
Consider comprehensive RTL testing across multiple components
The codebase has extensive usage of dir="auto" across 20 components, indicating a systematic approach to RTL support. The change to Title.tsx aligns with the existing RTL implementation pattern.
Key areas requiring RTL testing:
- User input components (Title.tsx, Description.tsx, Field.tsx)
- Display components (DisputePreview, DisputeView)
- Interactive components (Search.tsx, EvidenceSearch.tsx)
- Voting-related components (JustificationArea.tsx, OptionsContainer.tsx)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# List components that need RTL testing
echo "Components requiring RTL testing:"
rg -l "dir=\"auto\"" | sort
Length of output: 1153
web/src/pages/Resolver/Briefing/Description.tsx (1)
40-40: LGTM! The dir="auto" addition is well-placed.
The addition of dir="auto" is appropriate for the description textarea, especially given the potential for mixed-language content in dispute descriptions.
web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/FormContact.tsx (1)
5-5: LGTM! Changes align with existing patterns.
The addition of dir="auto" appropriately handles text direction for contact input fields. The isEmpty import follows the established pattern of using it after isUndefined checks.
Also applies to: 62-62
web/src/components/DisputeView/DisputeCardView.tsx (1)
11-11: LGTM! Language direction handling properly implemented
The addition of dir="auto" to the title element is appropriate for handling mixed-language content, and the restored responsiveSize import is correctly used in the component styling.
Also applies to: 40-40
web/src/components/Field.tsx (1)
103-103: LGTM! Language direction handling properly implemented
The addition of dir="auto" to the FieldContainer is appropriate for handling mixed-language content in field values. The implementation is consistent with other components in the PR.
web/src/components/CasesDisplay/Search.tsx (1)
93-93: LGTM! Verify RTL support in base component
The addition of dir="auto" is appropriate for handling mixed-direction text input. However, since this is a styled version of the base Searchbar component, it would be good to verify that the underlying component properly supports RTL text rendering.
Let's check if the base Searchbar component has RTL support:
web/src/components/DisputePreview/DisputeContext.tsx (3)
79-81: LGTM! Appropriate direction handling for dispute titles
The addition of dir="auto" to StyledH1 is correct for handling dispute titles that may contain mixed-language content.
85-92: LGTM! Proper text direction support for markdown content
Adding dir="auto" to ReactMarkdownWrapper is essential for proper rendering of markdown content that may contain mixed-language text. The implementation correctly wraps both question and description sections.
106-106: LGTM! Consistent direction handling for answers
The addition of dir="auto" to AnswerTitleAndDescription maintains consistency with other text components and properly handles mixed-language content in voting options.
web/src/pages/Cases/CaseDetails/Evidence/SubmitEvidenceModal.tsx (1)
95-100: LGTM! Appropriate text direction support for evidence input
The addition of dir="auto" to StyledTextArea is correct for handling evidence submissions that may contain mixed-language content. The implementation preserves the existing functionality while adding proper text direction support.
web/src/pages/Cases/CaseDetails/Voting/VotesDetails/index.tsx (3)
53-56: LGTM: Well-structured layout wrapper component
The new LabelWrapper component with flex layout and gap provides consistent spacing and alignment for label-text pairs.
62-62: LGTM: Proper flex distribution
The flex: 1 property on text labels ensures they take up available space while maintaining layout consistency.
Also applies to: 73-73
58-62: Verify text direction implementation for all user-generated content
The dir="auto" attribute is correctly implemented for user inputs (justification text and vote choice). However, let's verify if there are any other user-generated text fields that might need this attribute.
Also applies to: 73-73, 85-85, 91-91
✅ Verification successful
Based on the search results, I can see several text-containing components and styled elements throughout the codebase that display user-generated content. Let me verify if they handle text direction appropriately.
All user-generated content components have proper text direction handling
The codebase has consistent implementation of dir="auto" attribute for user-generated content across all relevant components:
- Evidence content and search
- Vote choices and justifications
- Case descriptions and titles
- Form inputs and text areas
- Dispute context and answers
- Markdown content wrappers
- Aliases and labels
The review comment's concern has been thoroughly verified, and all components that display user-generated text properly handle text direction.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find potential text fields that might need dir="auto"
# Look for props or variables that might contain user input
# Search for text-related props and state variables
rg -A 2 "(?i)(text|content|description|message|title|label|value)(\s*:|=)" --type typescript
# Search for styled-components that might render text
ast-grep --pattern 'styled.$_{
$$$
}'
Length of output: 171
Script:
#!/bin/bash
# Search for text-related styled components and their usage
rg -t tsx -t ts "(?i)(text|content|description|message|title|label|value)(\s*:|=)" -A 2
# Search for styled components that might render text
rg -t tsx -t ts "styled\.[a-zA-Z]+\s*=\s*styled" -A 2
# Look for potential text containers
rg -t tsx -t ts "<[^>]*(Text|Label|Message|Description|Content|Title)[^>]*>" -A 2
Length of output: 313
Script:
#!/bin/bash
# Search for text-related styled components and their usage
rg "(?i)(text|content|description|message|title|label|value)(\s*:|=)" --type-add 'typescript:*.{ts,tsx}' -t typescript -A 2
# Search for styled components that might render text
rg "styled\.[a-zA-Z]+\s*=\s*styled" --type-add 'typescript:*.{ts,tsx}' -t typescript -A 2
# Look for potential text containers
rg "<[^>]*(Text|Label|Message|Description|Content|Title)[^>]*>" --type-add 'typescript:*.{ts,tsx}' -t typescript -A 2
Length of output: 88761
Script:
#!/bin/bash
# Search for dir="auto" attribute usage
rg 'dir="auto"' --type-add 'typescript:*.{ts,tsx}' -t typescript
# Search for specific text components that might need dir="auto"
rg '<(StyledField|StyledTextArea|StyledLabel|JustificationText)[^>]*>' --type-add 'typescript:*.{ts,tsx}' -t typescript
Length of output: 8084
web/src/components/Verdict/FinalDecision.tsx (3)
40-47: LGTM: Improved layout distribution
The addition of flex: 1 to both the container and its child divs ensures proper space distribution and consistent layout.
105-105: LGTM: Fixed missing dependency
Good catch adding isDisconnected to the buttonText dependencies array, preventing potential stale closure issues.
27-29: Verify text direction in imported components
The summary mentions that AnswerDisplay has been updated with dir="auto". Let's verify its implementation and check if other text-displaying components need similar updates.
✅ Verification successful
Let me gather more specific information about the text direction implementation.
Let me check the related components that display text content to ensure consistent text direction handling.
Text direction is properly implemented across components
The verification shows that text direction is consistently handled:
AnswerDisplayusesdir="auto"through theAnswerTitleAndDescriptionstyled componentDisputeContext.tsximplementsdir="auto"for all text content including:- Main title (
StyledH1) - Question and description (
ReactMarkdownWrapper) - Answer options (
AnswerTitleAndDescription)
- Main title (
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify text direction implementation in related components
# Check AnswerDisplay implementation
rg -A 10 "export.*Answer" --type typescript
# Look for similar verdict-related components that might need dir="auto"
fd -e tsx -e ts . | xargs rg -l "Verdict|Answer|Decision"
Length of output: 1337
Script:
#!/bin/bash
# Check Answer.tsx implementation for dir="auto"
cat "web/src/components/Verdict/Answer.tsx"
# Check other verdict components for text direction handling
cat "web/src/components/Verdict/FinalDecision.tsx"
Length of output: 5905
Script:
#!/bin/bash
# Check the DisputeContext component that contains AnswerTitle and AnswerDescription
cat "web/src/components/DisputePreview/DisputeContext.tsx"
Length of output: 3731
✅ Deploy Preview for kleros-v2-university ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for kleros-v2-neo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for kleros-v2-testnet-devtools ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for kleros-v2-testnet-devtools ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Harman-singh-waraich
dismissed stale reviews from alcercu and coderabbitai[bot]
via
2f055e2
|
Code Climate has analyzed commit 2f055e2 and detected 5 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
|
PR-Codex overview
This PR focuses on enhancing accessibility by adding the
dir="auto"attribute to various components, ensuring proper text direction based on content. This change improves the user experience for languages that read right-to-left.Detailed summary
dir="auto"to multipleStyledField,StyledSearchBar, andStyledTextAreacomponents.AnswerTitleAndDescriptionandJustificationTextcomponents to support text direction.ReactMarkdownWrapperwithdir="auto"for better text rendering.Summary by CodeRabbit
New Features
dir="auto") for various input components across the application, enhancing support for multilingual content.Bug Fixes
SubmitEvidenceModalfor better debugging during evidence submission.Style
Chores