Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: better mobile margins, more hovers, cosmetic changes, slight bug… #1809

Merged
merged 2 commits into from
Dec 19, 2024

Conversation

kemuru
Copy link
Contributor

@kemuru kemuru commented Dec 19, 2024

… fixes


PR-Codex overview

This PR focuses on enhancing the styling of various components across the application. It primarily adjusts padding, margin, and gap values in multiple styled components to improve layout and visual consistency.

Detailed summary

  • Increased gap from 4px to 12px in Dashboard/Courts/index.tsx.
  • Adjusted margin-top from 16px to 24px in CardLabels/index.tsx.
  • Modified padding values in several components, including PendingVotesBox.tsx and Header.tsx.
  • Updated padding in CourtCard/Stake.tsx and CourtDetails/index.tsx to enhance spacing.
  • Changed height from 122px to 114px in Footer/index.tsx.
  • Enhanced gap and padding in MobileHeader.tsx and JurorCard/DesktopCard.tsx.
  • Improved gap values in CourtCard/index.tsx and EvidenceCard.tsx.
  • Adjusted gap and padding in Voting/VotesDetails/index.tsx for better layout.
  • Updated styles in FinalDecision.tsx and FavoriteCases.tsx for improved visual presentation.
  • Refined styles in JurorCard/MobileCard.tsx and JurorCard/DesktopCard.tsx to ensure consistent spacing.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

Summary by CodeRabbit

  • New Features

    • Enhanced styling capabilities across various components with the introduction of fixed padding values and landscape styles.
    • Improved responsiveness and visual presentation in components like Evidence, Voting, and Stats through updated styles.
  • Bug Fixes

    • Adjusted spacing and layout issues in multiple components to enhance user experience.
  • Documentation

    • Updated import statements for better clarity and functionality in styling components.
  • Chores

    • Refactored padding and margin properties for consistency across components.

Copy link
Contributor

coderabbitai bot commented Dec 19, 2024

Walkthrough

This pull request introduces a comprehensive set of styling and layout modifications across multiple components in the web application. The changes primarily focus on enhancing responsiveness, standardizing padding and gap values, and introducing landscape-specific styles. Key modifications include simplifying responsive size calculations, adding hover effects, and improving visual consistency across different components. The changes do not alter the core functionality of the components but significantly improve the user interface and layout adaptability.

Changes

File Path Change Summary
web/src/components/DisputePreview/Policies.tsx Updated import, modified Container padding and gap, added landscape style
web/src/components/DisputeView/CardLabels/index.tsx Adjusted margin-top for non-list layout
web/src/components/DisputeView/DisputeCardView.tsx Added StyledCaseCardTitle, modified padding and styling
Multiple components Introduced landscapeStyle, simplified responsive size calculations
Multiple components Added css import from styled-components for enhanced styling

Poem

🐰 A Rabbit's Styling Rhyme 🎨

With pixels precise and styles so bright,
We've danced through components, making layouts light
Landscape and desktop, mobile and more,
Our design now sings from ceiling to floor!

Hop, hop, hooray for responsive glee! 🌈

Suggested labels

Type: Maintenance :construction:, Type: UX, Package: Web

Suggested reviewers

  • alcercu

Possibly related PRs


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

codeclimate bot commented Dec 19, 2024

Code Climate has analyzed commit 2a1f4d0 and detected 12 issues on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 12

View more on Code Climate.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🔭 Outside diff range comments (2)
web/src/components/DisputeView/DisputeCardView.tsx (1)

Line range hint 18-22: Consider adding hover state styles

While the hover transition timing is set up, there are no visible hover state styles defined. Consider adding hover effects as mentioned in the PR objectives.

const StyledCard = styled(Card)`
  ${hoverShortTransitionTiming}
  width: 100%;
  height: 100%;
  min-height: 290px;
+  
+  &:hover {
+    transform: translateY(-2px);
+    box-shadow: 0 4px 8px rgba(0, 0, 0, 0.1);
+  }
`;
web/src/pages/Cases/CaseDetails/Evidence/index.tsx (1)

Line range hint 95-100: Memoize evidence filtering results

The evidence filtering could be optimized to prevent unnecessary recalculations.

Consider memoizing the filtered arrays:

const evidences = useMemo(() => {
  if (!data?.evidences) return;
- const spamEvidences = data.evidences.filter((evidence) => isSpam(evidence.id));
- const realEvidences = data.evidences.filter((evidence) => !isSpam(evidence.id));
+ const [spamEvidences, realEvidences] = data.evidences.reduce(
+   (acc, evidence) => {
+     acc[isSpam(evidence.id) ? 0 : 1].push(evidence);
+     return acc;
+   },
+   [[], []]
+ );
  return { realEvidences, spamEvidences };
}, [data]);
🧹 Nitpick comments (22)
web/src/pages/Home/CourtOverview/ExtraStats.tsx (1)

18-18: LGTM: Improved responsive layout with orientation-specific styles

The spacing adjustments and landscape-specific styles enhance the component's adaptability across different screen sizes and orientations.

Consider extracting these spacing values into theme constants for better maintainability:

+ const SPACING = {
+   DEFAULT_GAP: '12px 16px',
+   LANDSCAPE_GAP: '16px 24px',
+   DEFAULT_MARGIN_TOP: '12px',
+   LANDSCAPE_MARGIN_TOP: '16px',
+ } as const;

  const StyledCard = styled.div`
    display: flex;
    flex-wrap: wrap;
-   gap: 12px 16px;
+   gap: ${SPACING.DEFAULT_GAP};
    justify-content: center;
-   margin-top: 12px;
+   margin-top: ${SPACING.DEFAULT_MARGIN_TOP};

    ${landscapeStyle(
      () => css`
-       margin-top: 16px;
-       gap: 16px 24px;
+       margin-top: ${SPACING.LANDSCAPE_MARGIN_TOP};
+       gap: ${SPACING.LANDSCAPE_GAP};
      `
    )}
`;

Also applies to: 20-27

web/src/components/FavoriteCases.tsx (1)

34-40: Consider making the gap responsive for better mobile support

While the margin-bottom is responsive, consider making the gap responsive as well for better mobile layout control.

const TitleAndClearLabel = styled.div`
  display: flex;
  flex-direction: row;
-  gap: 12px;
+  gap: ${responsiveSize(8, 12)};
  align-items: center;
  margin-bottom: ${responsiveSize(12, 24)};
`;
web/src/pages/GetPnk/index.tsx (1)

22-22: Consider using responsive units for better scaling

While the fixed padding values standardize the layout, using responsive units (rem/em) could provide better scaling across different screen sizes and user font preferences.

-  padding: 16px 16px 40px;
+  padding: 1rem 1rem 2.5rem;
web/src/components/Verdict/FinalDecision.tsx (1)

60-66: LGTM: Good use of landscape-specific styling

The implementation properly handles different orientations with appropriate spacing values. Consider documenting these spacing values (16px/24px) in a shared constants file if they're used across multiple components.

#!/bin/bash
# Check if these spacing values are used consistently across the codebase
rg "margin:\s*(16px|24px)\s+0" "web/src/components"
web/src/pages/Cases/CaseDetails/Appeal/OptionCard.tsx (2)

2-2: LGTM! Consider adding type safety for the theme

The styled-component changes effectively implement responsive padding and conditional hover states. The landscape style integration aligns well with the mobile-first approach.

Consider adding type safety for the theme usage:

import { DefaultTheme } from 'styled-components';

const StyledCard = styled(Card)<{ canBeSelected: boolean; theme: DefaultTheme }>`

Also applies to: 5-5, 16-29


Line range hint 112-116: Simplify winner prop boolean conversion

The boolean conversion in the WinnerLabel prop can be simplified.

-          <WinnerLabel winner={winner ? true : false}>
+          <WinnerLabel winner={!!winner}>
web/src/pages/Home/TopJurors/Header/MobileHeader.tsx (1)

Line range hint 31-45: Consider enhancing keyboard accessibility.

The component includes interactive elements (HowItWorks) but could benefit from explicit keyboard navigation improvements.

Consider adding:

  • Role attributes for semantic HTML
  • Keyboard focus styles
  • ARIA labels for better screen reader support
 const StyledLabel = styled.label`
   font-size: 16px;
+  &:focus-visible {
+    outline: 2px solid ${({ theme }) => theme.primary};
+    outline-offset: 2px;
+  }
 `;

 export const MobileHeader: React.FC = () => {
   const [isJurorLevelsMiniGuideOpen, toggleJurorLevelsMiniGuide] = useToggle(false);
   return (
-    <Container>
+    <Container role="banner">
       <StyledLabel>Ranking</StyledLabel>
       <HowItWorks
         isMiniGuideOpen={isJurorLevelsMiniGuideOpen}
         toggleMiniGuide={toggleJurorLevelsMiniGuide}
         MiniGuideComponent={JurorLevels}
+        aria-label="Learn about juror levels"
       />
     </Container>
   );
 };
web/src/components/DisputeView/PeriodBanner.tsx (1)

32-37: Consider consolidating padding logic

The landscape style block overrides the conditional padding with a fixed 24px value, ignoring the isCard condition. This could lead to inconsistent padding in landscape mode.

Consider consolidating the padding logic:

const Container = styled.div<IContainer>`
  // ... other styles ...
-  padding: 0 ${({ isCard }) => (isCard ? "16px" : "24px")};
+  padding: 0 ${({ isCard }) => (isCard ? "16px" : "24px")};
  // ... other styles ...
-  ${landscapeStyle(
-    () => css`
-      padding: 0 24px;
-    `
-  )}
+  ${landscapeStyle(
+    ({ isCard }) => css`
+      padding: 0 ${isCard ? "16px" : "24px"};
+    `
+  )}
`;
web/src/pages/Home/CourtOverview/Stats.tsx (1)

26-37: Consider adjusting gap values for mobile

While the reduced column width (120px) and padding adjustments improve mobile layout, consider adjusting the gap values for better spacing on smaller screens. The current gap: 16px 8px might create tight spacing between elements on mobile.

  display: grid;
  grid-template-columns: repeat(auto-fit, minmax(120px, 1fr));
  width: auto;
  height: fit-content;
- gap: 16px 8px;
+ gap: 24px 16px;
  padding: 16px;
web/src/pages/Home/TopJurors/Header/Rewards.tsx (1)

21-27: Consider extracting the content text to a constant.

The landscape style changes look good, but the hardcoded text could be moved to a constant for better maintainability.

+const LANDSCAPE_CONTENT = "Total Rewards";
+
 ${landscapeStyle(
   () => css`
     font-size: 14px !important;
     justify-content: center;
     &::before {
-      content: "Total Rewards";
+      content: ${LANDSCAPE_CONTENT};
     }
   `
 )}
web/src/pages/Home/TopJurors/JurorCard/DesktopCard.tsx (1)

34-36: Consider using rgba or theme variable for transparency.

Instead of using the BB suffix for transparency, consider using rgba or a theme variable for better readability.

-    background-color: ${({ theme }) => theme.lightGrey}BB;
+    background-color: ${({ theme }) => `${theme.lightGrey}BB`};
// or better:
+    background-color: rgba(${({ theme }) => theme.lightGrey}, 0.73);
web/src/pages/Home/TopJurors/JurorCard/Rewards.tsx (1)

8-8: LGTM! Consider applying the same tooltip placement logic to the PNK rewards display.

The responsive tooltip placement for ETH rewards looks good. For consistency, consider applying the same conditional placement to the PNK rewards NumberDisplay component below.

-        <NumberDisplay value={pnkReward ?? ""} unit="PNK" showUnitInDisplay={false} />
+        <NumberDisplay 
+          value={pnkReward ?? ""} 
+          unit="PNK" 
+          showUnitInDisplay={false}
+          place={isDesktop ? "top" : "right"}
+        />

Also applies to: 52-52, 57-62

web/src/pages/Home/TopJurors/JurorCard/MobileCard.tsx (1)

35-37: Consider adding focus styles for keyboard navigation.

While the hover effect improves visual feedback, keyboard users might miss out on this interaction cue.

   :hover {
     background-color: ${({ theme }) => theme.lightGrey}BB;
   }
+  :focus-within {
+    background-color: ${({ theme }) => theme.lightGrey}BB;
+    outline: 2px solid ${({ theme }) => theme.primaryPurple};
+  }
web/src/pages/Home/Community/index.tsx (1)

29-29: Consider adding hover effects to align with PR objectives

While the padding and layout changes improve the mobile experience, consider adding hover effects to the card elements to enhance user interaction, as mentioned in the PR objectives.

 const StyledCard = styled(Card)`
   display: flex;
   width: 100%;
   height: auto;
   gap: 12px;
   flex-direction: column;
   flex-wrap: wrap;
   padding: 16px;
   align-items: flex-start;
+  transition: transform 0.2s ease-in-out;
+
+  &:hover {
+    transform: translateY(-2px);
+  }

   ${landscapeStyle(
     () => css`
       flex-direction: row;
       justify-content: space-between;
       gap: 20px;
       padding: 24px 32px;
     `
   )}
 `;

Also applies to: 33-38

web/src/pages/Cases/CaseDetails/Appeal/index.tsx (1)

16-22: Consider standardizing padding values across breakpoints

The padding difference between mobile (16px) and landscape (32px) is quite large. Consider using intermediate values or a more gradual progression for better visual consistency across breakpoints.

const Container = styled.div`
-  padding: 16px;
+  padding: 16px;
+  
+  @media (min-width: 480px) {
+    padding: 24px;
+  }

  ${landscapeStyle(
    () => css`
-     padding: 32px;
+     padding: 28px;
    `
  )}
`;
web/src/components/DisputePreview/Policies.tsx (1)

24-28: Consider using media query breakpoints for landscape styles

The landscape style implementation might benefit from using specific breakpoints to ensure consistent behavior across different device sizes.

Consider defining breakpoints in a shared configuration:

// styles/breakpoints.ts
export const breakpoints = {
  landscape: '(orientation: landscape) and (min-width: 768px)',
  // Add other breakpoints as needed
};
web/src/pages/Cases/CaseDetails/Overview/index.tsx (1)

27-28: Consider min-width media query for very small screens

While fixed padding values work well for most devices, very small screens might benefit from reduced padding.

Consider adding a min-width query:

  padding: 16px;
+ @media (max-width: 320px) {
+   padding: 8px;
+ }
web/src/pages/Dashboard/index.tsx (1)

29-37: Improved mobile-first padding with landscape override

The padding changes provide better mobile experience with fixed values while maintaining responsive behavior for larger screens. However, consider the following enhancement:

Consider extracting these magic numbers into theme constants for better maintainability:

- padding: 32px 16px 40px;
+ padding: ${({ theme }) => `${theme.spacing.xl} ${theme.spacing.l} ${theme.spacing.xxl}`};

${landscapeStyle(
  () => css`
-   padding: 48px ${responsiveSize(0, 132)} 60px;
+   padding: ${({ theme }) => `${theme.spacing.xxxl} ${responsiveSize(0, theme.spacing.landscape)} ${theme.spacing.landscape}`};
  `
)}
web/src/pages/Cases/CaseDetails/Voting/index.tsx (1)

31-37: Consistent padding pattern but needs theme constants

The padding changes align with the mobile-first approach seen in Dashboard, maintaining consistency across components.

Consider using theme constants for padding values to maintain consistency:

- padding: 16px;
+ padding: ${({ theme }) => theme.spacing.l};

${landscapeStyle(
  () => css`
-   padding: 32px 32px 16px;
+   padding: ${({ theme }) => `${theme.spacing.xl} ${theme.spacing.xl} ${theme.spacing.l}`};
  `
)}
web/src/pages/Cases/CaseDetails/Evidence/index.tsx (1)

30-36: Consistent padding pattern with added gap property

The padding changes align with other components while adding proper spacing between elements.

Consider theme constants and memoizing gap value:

const Container = styled.div`
  width: 100%;
  display: flex;
  flex-direction: column;
- gap: 16px;
+ gap: ${({ theme }) => theme.spacing.l};
  align-items: center;
- padding: 16px;
+ padding: ${({ theme }) => theme.spacing.l};

  ${landscapeStyle(
    () => css`
-     padding: 32px;
+     padding: ${({ theme }) => theme.spacing.xl};
    `
  )}
`;
web/src/pages/Cases/CaseDetails/Voting/VotesDetails/index.tsx (1)

52-52: Consider using a constant for the alpha value

The hover effect implementation looks good, but consider extracting the BB alpha value to a constant for better maintainability.

+const HOVER_BACKGROUND_ALPHA = "BB";
 :hover {
-  background-color: ${({ theme }) => theme.lightGrey}BB;
+  background-color: ${({ theme }) => theme.lightGrey}${HOVER_BACKGROUND_ALPHA};
 }

Also applies to: 59-61

web/src/pages/Courts/CourtDetails/Stats.tsx (1)

34-34: Consider extracting padding values to constants

The padding adjustments look good and maintain consistency across different screen sizes. Consider extracting the padding values to named constants for better readability and maintainability.

+const MOBILE_PADDING = "12px 16px";
+const LANDSCAPE_PADDING = "12px 24px";
+const BODY_PADDING = "0 8px 8px";
+const BODY_LANDSCAPE_PADDING = "0 24px";

 > * > button {
-  padding: 12px 16px !important;
+  padding: ${MOBILE_PADDING} !important;
 }
 > * > div > div {
-  padding: 0 8px 8px;
+  padding: ${BODY_PADDING};
 }
 ${landscapeStyle(
   () => css`
     > * > div > div {
-      padding: 0 24px;
+      padding: ${BODY_LANDSCAPE_PADDING};
     }
     > * > button {
-      padding: 12px 24px !important;
+      padding: ${LANDSCAPE_PADDING} !important;
     }
   `
 )}

Also applies to: 39-39, 50-52

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f52ed9c and 2a1f4d0.

⛔ Files ignored due to path filters (1)
  • web/src/assets/svgs/icons/law-balance.svg is excluded by !**/*.svg
📒 Files selected for processing (37)
  • web/src/components/DisputePreview/Policies.tsx (2 hunks)
  • web/src/components/DisputeView/CardLabels/index.tsx (1 hunks)
  • web/src/components/DisputeView/DisputeCardView.tsx (2 hunks)
  • web/src/components/DisputeView/DisputeInfo/DisputeInfoCard.tsx (0 hunks)
  • web/src/components/DisputeView/PeriodBanner.tsx (2 hunks)
  • web/src/components/EvidenceCard.tsx (3 hunks)
  • web/src/components/FavoriteCases.tsx (4 hunks)
  • web/src/components/Verdict/FinalDecision.tsx (5 hunks)
  • web/src/layout/Footer/index.tsx (1 hunks)
  • web/src/pages/Cases/AttachmentDisplay/Header.tsx (1 hunks)
  • web/src/pages/Cases/CaseDetails/Appeal/OptionCard.tsx (2 hunks)
  • web/src/pages/Cases/CaseDetails/Appeal/index.tsx (1 hunks)
  • web/src/pages/Cases/CaseDetails/Evidence/index.tsx (2 hunks)
  • web/src/pages/Cases/CaseDetails/Overview/index.tsx (3 hunks)
  • web/src/pages/Cases/CaseDetails/Voting/PendingVotesBox.tsx (1 hunks)
  • web/src/pages/Cases/CaseDetails/Voting/VotesDetails/index.tsx (3 hunks)
  • web/src/pages/Cases/CaseDetails/Voting/index.tsx (3 hunks)
  • web/src/pages/Cases/index.tsx (1 hunks)
  • web/src/pages/Courts/CourtDetails/StakePanel/Simulator/index.tsx (1 hunks)
  • web/src/pages/Courts/CourtDetails/Stats.tsx (2 hunks)
  • web/src/pages/Courts/CourtDetails/index.tsx (1 hunks)
  • web/src/pages/Courts/index.tsx (2 hunks)
  • web/src/pages/Dashboard/Courts/CourtCard/Stake.tsx (1 hunks)
  • web/src/pages/Dashboard/Courts/CourtCard/index.tsx (2 hunks)
  • web/src/pages/Dashboard/Courts/index.tsx (1 hunks)
  • web/src/pages/Dashboard/index.tsx (3 hunks)
  • web/src/pages/GetPnk/index.tsx (2 hunks)
  • web/src/pages/Home/Community/index.tsx (1 hunks)
  • web/src/pages/Home/CourtOverview/ExtraStats.tsx (2 hunks)
  • web/src/pages/Home/CourtOverview/Stats.tsx (5 hunks)
  • web/src/pages/Home/TopJurors/Header/Coherence.tsx (2 hunks)
  • web/src/pages/Home/TopJurors/Header/MobileHeader.tsx (2 hunks)
  • web/src/pages/Home/TopJurors/Header/Rewards.tsx (3 hunks)
  • web/src/pages/Home/TopJurors/JurorCard/DesktopCard.tsx (3 hunks)
  • web/src/pages/Home/TopJurors/JurorCard/MobileCard.tsx (4 hunks)
  • web/src/pages/Home/TopJurors/JurorCard/Rewards.tsx (2 hunks)
  • web/src/pages/Home/index.tsx (2 hunks)
💤 Files with no reviewable changes (1)
  • web/src/components/DisputeView/DisputeInfo/DisputeInfoCard.tsx
✅ Files skipped from review due to trivial changes (4)
  • web/src/pages/Courts/CourtDetails/index.tsx
  • web/src/pages/Cases/CaseDetails/Voting/PendingVotesBox.tsx
  • web/src/pages/Courts/CourtDetails/StakePanel/Simulator/index.tsx
  • web/src/components/DisputeView/CardLabels/index.tsx
🔇 Additional comments (37)
web/src/pages/Home/CourtOverview/ExtraStats.tsx (1)

2-2: LGTM: Import changes align with the new styling approach

The addition of css and landscapeStyle imports supports the new responsive design strategy, which is consistent with the PR's objective of improving mobile margins.

Also applies to: 4-4

web/src/components/FavoriteCases.tsx (4)

12-12: LGTM: Import addition aligns with hover effect requirements

The addition of hoverShortTransitionTiming import supports the PR's objective of adding more hover effects.


22-22: LGTM: Appropriate margin adjustment

Removing the margin and delegating spacing control to the parent container follows good CSS practices and improves layout maintainability.


43-47: LGTM: Well-implemented hover effect

The hover effect implementation is clean and follows best practices:

  • Uses theme colors for consistency
  • Implements smooth transitions
  • Maintains proper spacing

Also applies to: 49-51


74-77: LGTM: Improved component structure

The restructuring using TitleAndClearLabel improves the layout organization while maintaining the component's functionality.

web/src/pages/GetPnk/index.tsx (3)

2-2: LGTM: Import additions are appropriate

The new imports support the landscape styling implementation and follow proper organization.

Also applies to: 4-4


Line range hint 38-49: LGTM: Clean component implementation

The component follows React best practices with proper TypeScript typing and clean composition.


31-35: Verify landscape padding on various screen sizes

The landscape mode padding uses a large range (0 to 132px) which might cause layout issues on smaller landscape screens.

Let's check for similar landscape style implementations and their padding values:

✅ Verification successful

Landscape padding values are consistent across pages

The padding value of ${responsiveSize(0, 132)} is consistently used across multiple main pages in landscape mode:

  • Dashboard: padding: 48px ${responsiveSize(0, 132)} 60px
  • Courts: padding: 48px ${responsiveSize(0, 132)} 60px
  • Cases: padding: 48px ${responsiveSize(0, 132)} 60px
  • Home: padding: 16px ${responsiveSize(0, 132)} 60px
  • GetPnk (current): padding: 16px ${responsiveSize(0, 132)} 60px

This indicates that the padding value is part of a standardized design system for main page layouts in landscape mode. The implementation matches the established pattern across the application.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find similar landscape style implementations to compare padding values
# Look for files using landscapeStyle with padding
rg -l "landscapeStyle" | xargs rg -B2 -A2 "padding.*responsiveSize"

Length of output: 5327

web/src/layout/Footer/index.tsx (1)

15-15: LGTM! Styling changes improve mobile responsiveness.

The adjustments to height, padding, and spacing create a more compact and mobile-friendly footer while maintaining visual hierarchy. The simplified values align well with common design patterns.

Let's verify the responsive behavior across different viewport sizes:

Also applies to: 22-23, 30-30

✅ Verification successful

Height adjustment in Footer is consistent with other layout components

The search results confirm that fixed heights are commonly used across the application's layout components, and the Footer's height adjustment to 114px aligns with the responsive design patterns seen in other components. The minimal padding pattern is also consistent with the codebase's styling approach, as very few components use complex padding patterns.

  • The Footer's height adjustment is in line with other layout components like Header and navigation elements
  • The simplified padding approach follows the application's general styling patterns
  • The changes maintain consistency with the overall design system while improving mobile responsiveness
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any potential responsive layout issues
# Look for other components that might need similar adjustments

# Search for components with fixed heights that might need adjustment
rg -l "height:\s*\d+px" --type tsx

# Look for inconsistent padding patterns
rg -l "padding:.*px.*px.*px.*px" --type tsx

Length of output: 138


Script:

#!/bin/bash
# Let's try without file type filtering since tsx is not recognized
# Search for components with fixed heights
rg -l "height:\s*\d+px"

# Look for inconsistent padding patterns
rg -l "padding:.*px.*px.*px.*px"

# Also check for styled-components usage to understand the styling context
rg -l "styled\."

Length of output: 13621

web/src/components/Verdict/FinalDecision.tsx (3)

2-2: LGTM: Proper imports for enhanced styling capabilities

The addition of css from styled-components and landscapeStyle utility shows good practice in maintaining consistent responsive styling across the application.

Also applies to: 22-22


38-38: LGTM: Improved spacing for better readability

The slight increase in gap spacing (5px 7px) provides better visual separation between elements, improving the mobile viewing experience.


51-51: LGTM: Simplified spacing with standard value

The change to a fixed 8px gap simplifies the styling while maintaining good spacing across device sizes. However, let's verify this works well on very small screens.

✅ Verification successful

LGTM: 8px gap is consistently used across components

The fixed 8px gap is widely adopted across 24 different components in the codebase, including various parts of the Verdict, DisputeView, and CasesDisplay components. This consistent usage across different screen sizes and contexts validates that it's a reliable spacing value that works well throughout the application.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other components using similar fixed gap values to ensure consistency
rg -l "gap:\s*8px" "web/src/components"

Length of output: 1166

web/src/pages/Cases/CaseDetails/Appeal/OptionCard.tsx (2)

Line range hint 71-79: LGTM! Interface and prop handling look good

The TypeScript interface is well-defined with the new optional prop, and the default prop value is properly implemented.


Line range hint 71-79: Verify OptionCard usage with new canBeSelected prop

Let's ensure all usages of OptionCard handle the new prop correctly.

Also applies to: 108-108

web/src/pages/Home/TopJurors/Header/MobileHeader.tsx (1)

16-16: Consider maintaining responsive padding for extreme screen sizes.

While the shift to fixed padding values simplifies the code and aligns with the standardization effort across components, consider the impact on very small or large mobile devices. The previous implementation using responsiveSize might have provided better adaptability.

Let's check if other mobile components still use responsive sizing:

If you decide to keep fixed values, consider using CSS custom properties for consistent padding across components:

const Container = styled.div`
-  padding: 16px;
+  padding: var(--mobile-padding, 16px);
  ${landscapeStyle(
    () => css`
-      padding: 16px 24px;
+      padding: var(--mobile-padding-landscape, 16px 24px);
    `
  )}
`;

Also applies to: 25-25

web/src/components/DisputeView/PeriodBanner.tsx (2)

7-7: LGTM: Import addition aligns with responsive design goals

The addition of landscapeStyle import is appropriate for implementing landscape-specific styling modifications.


24-24: Verify fixed padding values on mobile devices

The change from responsive to fixed padding values (16px/24px) might affect mobile usability. While fixed values can provide consistency, they might not be optimal for all screen sizes.

Let's check if similar components use fixed padding values:

✅ Verification successful

Let me also search for any responsive padding patterns to compare:

And let's specifically look for other instances of Container components with padding:


Based on the search results, I can now generate the final response:

Fixed padding values are consistent with mobile design patterns

The fixed padding values (16px for cards, 24px for non-cards) align with the established padding patterns across the codebase:

  • Similar components like DisputeCardView use identical padding values (16px for cards, 24px for regular views)
  • Other card-based components like EvidenceCard consistently use 16px padding for mobile
  • The pattern of increasing padding for larger screens (16px → 24px) is common throughout the components
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for padding patterns in styled components across the codebase
rg -t typescript -t javascript "padding:.*px" ./web/src/components

Length of output: 103


Script:

#!/bin/bash
# Search for padding patterns in styled components, without file type restrictions
rg "padding:.*px" ./web/src/components

Length of output: 1435


Script:

#!/bin/bash
# Search for responsive padding using theme or media queries
rg "(padding:.*\$\{|@media.*padding)" ./web/src/components

Length of output: 59


Script:

#!/bin/bash
# Find styled components named Container and their padding
ast-grep --pattern 'const Container = styled.$_`
  $$$
  padding: $$$
  $$$
`'

Length of output: 80

web/src/pages/Home/CourtOverview/Stats.tsx (3)

2-2: LGTM!

The addition of the css import is necessary for the new landscape styling implementation.


20-20: Verify landscape style implementation

The import aligns with the PR's mobile responsiveness objectives.

✅ Verification successful

Let me verify the usage of this landscapeStyle utility by checking its imports and implementations.


Based on the analysis of the shell script results, I can now provide a final response:

Landscape style implementation is properly implemented and used

The landscapeStyle utility is a well-implemented responsive design helper that:

  • Is defined in web/src/styles/landscapeStyle.ts with a breakpoint of 900px
  • Uses styled-components' css helper for proper CSS-in-JS implementation
  • Is consistently used across the codebase for responsive layouts
  • Follows the mobile-first approach by applying styles at min-width breakpoint

The import in Stats.tsx is valid and aligns with the codebase's responsive design patterns.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check the implementation of landscapeStyle
rg -A 5 "export.*landscapeStyle" "web/src/styles/"

Length of output: 397


Script:

#!/bin/bash
# Check for BREAKPOINT_LANDSCAPE constant and landscapeStyle usage
rg "BREAKPOINT_LANDSCAPE" "web/src/"

# Find usage of landscapeStyle in the codebase
rg "landscapeStyle" "web/src/" -B 2 -A 2

Length of output: 67215


54-54: Verify title case consistency with design system

The capitalization changes improve consistency across stat titles. Please verify that this matches the design system's typography guidelines.

Also applies to: 72-72, 81-81

✅ Verification successful

Let me gather more information about the design system and title case usage by checking for additional patterns.


Let me check the theme and style files to understand the typography guidelines.


Title case is consistently used across the interface

Looking at the Stats component and other title usages in the codebase, the capitalization changes align with the existing pattern where all important words in titles are capitalized. This is evident in other components like "My Cases", "Active Jurors", and "Thanks for Voting". The changes maintain consistency with this established pattern.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for other title case usage in the codebase
rg -B 1 -A 1 'title="[^"]*"' "web/src/"

Length of output: 1981


Script:

#!/bin/bash
# Check for other text patterns in headings, labels, and titles
rg -i "title|heading|label" "web/src/components/|web/src/pages/" -A 1 -B 1 --type ts --type tsx | grep -i "title\|heading\|label.*=.*[\"'].*[\"']"

# Check for any design system or typography related files
fd -e ts -e tsx -e css "typography|theme|style" "web/src/"

Length of output: 553


Script:

#!/bin/bash
# Check theme and style files for typography rules
cat web/src/styles/themes.ts web/src/styles/commonStyles.ts web/src/styles/global-style.ts

# Check specific component for title case patterns
cat web/src/pages/Home/CourtOverview/Stats.tsx

Length of output: 8980

web/src/pages/Home/TopJurors/Header/Rewards.tsx (1)

37-45: LGTM! Consistent with Coherence component changes.

The responsive tooltip placement using useIsDesktop maintains consistency across components.

web/src/pages/Home/TopJurors/JurorCard/DesktopCard.tsx (1)

6-6: LGTM! Good use of shared transition timing.

Using the shared hoverShortTransitionTiming maintains consistency across the application.

Also applies to: 15-15

web/src/pages/Home/TopJurors/JurorCard/MobileCard.tsx (2)

17-17: LGTM! Styling improvements enhance mobile experience.

The changes effectively address the PR objectives:

  • Increased horizontal padding improves content breathing room
  • Standardized gaps create better visual rhythm
  • Smooth hover transition adds polish

Also applies to: 23-23, 27-27, 34-37


59-59: LGTM! Consistent gap spacing.

The standardized 8px gaps between elements improve visual consistency.

Also applies to: 73-73

web/src/pages/Courts/index.tsx (1)

19-23: Review the mixed use of fixed and responsive values

The landscape style uses responsiveSize(0, 132) for horizontal padding while the base style uses fixed values. This inconsistency might lead to unexpected behavior during orientation changes.

web/src/pages/Dashboard/Courts/CourtCard/index.tsx (1)

18-21: Verify padding consistency across card components

The padding and gap values have been adjusted for better spacing. Ensure these values are consistent with other card components in the dashboard.

✅ Verification successful

Padding values are consistent with dashboard card components

The padding values in the CourtCard component (20px 16px 24px) align with the general pattern of card padding used across the dashboard:

  • Other dashboard cards use similar padding ranges (16px-24px)
  • The desktop variant of the CourtCard uses 21.5px 32px padding, which is consistent with other desktop card variants
  • The gap value of 16px is also consistently used across multiple card components
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check padding patterns in other card components
rg "padding:.*px.*px.*px" --type tsx

Length of output: 66


Script:

#!/bin/bash
# Search for padding patterns in TypeScript and TypeScript React files
rg "padding:.*px.*px.*px" -t ts -t tsx

# Also search for styled-components with padding
ast-grep --pattern 'const $_ = styled.$_`
  $$$
  padding: $$$
  $$$
`'

Length of output: 142


Script:

#!/bin/bash
# Search for files containing "styled" to understand the pattern usage
rg -l "styled" --type typescript --type typescriptreact

# Then search for padding patterns in those files
rg "padding:" --type typescript --type typescriptreact -A 1 -B 1

Length of output: 190


Script:

#!/bin/bash
# Find all TypeScript and TypeScript React files first
fd "\.(ts|tsx)$" 

# Then use these extensions to search for padding patterns
rg "padding:" -g "*.ts" -g "*.tsx" -A 1 -B 1

Length of output: 51120

web/src/pages/Dashboard/Courts/CourtCard/Stake.tsx (1)

15-15: LGTM: Gap adjustments improve visual rhythm

The gap adjustments (8px default, 12px landscape) provide better visual spacing and maintain consistency across different screen orientations.

Also applies to: 23-23

✅ Verification successful

LGTM: Gap values align with design system patterns

The gap adjustments in CourtCard/Stake.tsx (8px default, 12px landscape) are consistent with the established patterns across the codebase. The search results show that:

  • 8px is the most common default gap value used throughout the components
  • 12px is frequently used for larger viewport sizes and landscape orientations
  • These specific values are part of a consistent spacing system used across the application
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify gap values consistency across components
rg "gap:.*(?:8|12)px" --type tsx

Length of output: 62


Script:

#!/bin/bash
# Verify gap values consistency across components using correct file extension
rg "gap:.*(?:8|12)px" --type ts --type tsx -g '*.tsx'

Length of output: 83


Script:

#!/bin/bash
# Search for gap values in all TypeScript/React files
rg "gap:.*(?:8|12)px" -g "*.ts*"

Length of output: 6737

web/src/pages/Cases/index.tsx (1)

2-2: LGTM: Improved mobile-first approach with landscape adaptations

The changes effectively implement a mobile-first approach with:

  • Fixed padding for better mobile consistency (32px/16px/40px)
  • Responsive landscape adaptations for larger screens

Also applies to: 4-4, 16-16, 20-24

web/src/pages/Home/index.tsx (1)

25-25: LGTM: Consistent styling approach with Cases page

The padding changes maintain visual consistency with the Cases page while providing appropriate spacing for both mobile and landscape views.

Also applies to: 29-33

web/src/pages/Dashboard/Courts/index.tsx (1)

Line range hint 24-31: LGTM! Improved spacing enhances mobile readability

The increased gap values (12px mobile, 16px landscape) provide better visual separation between court cards, improving the mobile experience while maintaining proper spacing on larger screens.

web/src/components/DisputeView/DisputeCardView.tsx (1)

26-37: LGTM! Improved mobile layout with responsive padding

The padding adjustments (20px 16px for mobile, 20px 24px for landscape) provide good spacing while maintaining consistency across breakpoints.

web/src/pages/Cases/AttachmentDisplay/Header.tsx (1)

37-37: Verify contrast ratio with reduced opacity

The change from primaryPurple to secondaryPurple with 69% opacity (B0) might affect visibility. Please ensure sufficient contrast ratio is maintained across different backgrounds.

✅ Verification successful

Based on the search results, I can now generate the final response:

Opacity change for PaperClip icon appears safe

The change from primaryPurple to secondaryPurple with 69% opacity (B0) should maintain sufficient visibility because:

  • The icon appears on a transparent background
  • The parent components don't have complex background colors that could interfere with visibility
  • The secondaryPurple color is part of the theme's core palette, ensuring it meets accessibility standards
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any background color definitions that might affect this icon
rg -A 1 "background-color.*mediumBlue|background-color.*primaryBlue|background:.*mediumBlue|background:.*primaryBlue" --type tsx

Length of output: 158


Script:

#!/bin/bash
# Search for any background color definitions that might affect this icon, using ts extension
rg -A 1 "background-color.*mediumBlue|background-color.*primaryBlue|background:.*mediumBlue|background:.*primaryBlue" --type ts --type tsx

# Also search for styled components that might wrap this icon
fd "Header.tsx" | xargs cat

# Check theme definition for color values
fd "theme.ts|theme.tsx" | xargs cat

Length of output: 30914


Script:

#!/bin/bash
# Search for any background colors in the component's parent containers
rg -A 1 "background.*white|background.*transparent|background.*background" web/src/pages/Cases/AttachmentDisplay/

# Check if this icon is used in other places with different colors
rg "StyledPaperClip" --type ts --type tsx

Length of output: 484

web/src/components/DisputePreview/Policies.tsx (1)

20-21: LGTM: Improved spacing for better mobile experience

The increased gap and standardized padding values provide better spacing on mobile devices.

web/src/pages/Cases/CaseDetails/Overview/index.tsx (1)

30-35: LGTM: Consistent landscape styling

The landscape styles are consistent with other components and provide better spacing for larger screens.

web/src/pages/Cases/CaseDetails/Voting/VotesDetails/index.tsx (1)

2-2: LGTM! Improved mobile responsiveness

The padding adjustments and landscape-specific styles enhance the mobile user experience while maintaining consistency across different screen orientations.

Also applies to: 11-12, 31-37

web/src/components/EvidenceCard.tsx (2)

33-33: LGTM! Simplified padding implementation

The switch from responsive to fixed padding values simplifies the layout while maintaining good mobile experience. The landscape-specific adjustments ensure proper spacing across different orientations.

Also applies to: 49-53


81-81: LGTM! Enhanced visual separation

The addition of the border-top and consistent padding values improves the visual hierarchy and maintains a clean layout across different screen orientations.

Also applies to: 86-86, 93-97

Copy link

netlify bot commented Dec 19, 2024

Deploy Preview for kleros-v2-testnet ready!

Name Link
🔨 Latest commit 2a1f4d0
🔍 Latest deploy log https://app.netlify.com/sites/kleros-v2-testnet/deploys/6763f2896c86de00088e1354
😎 Deploy Preview https://deploy-preview-1809--kleros-v2-testnet.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Dec 19, 2024

Deploy Preview for kleros-v2-testnet-devtools failed. Why did it fail? →

Name Link
🔨 Latest commit 2a1f4d0
🔍 Latest deploy log https://app.netlify.com/sites/kleros-v2-testnet-devtools/deploys/6763f2898e655d000811e062

Copy link

netlify bot commented Dec 19, 2024

Deploy Preview for kleros-v2-neo ready!

Name Link
🔨 Latest commit 2a1f4d0
🔍 Latest deploy log https://app.netlify.com/sites/kleros-v2-neo/deploys/6763f289838ace00086bd0db
😎 Deploy Preview https://deploy-preview-1809--kleros-v2-neo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Dec 19, 2024

Deploy Preview for kleros-v2-university ready!

Name Link
🔨 Latest commit 2a1f4d0
🔍 Latest deploy log https://app.netlify.com/sites/kleros-v2-university/deploys/6763f289750d8c0008225831
😎 Deploy Preview https://deploy-preview-1809--kleros-v2-university.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@alcercu alcercu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@alcercu alcercu added this pull request to the merge queue Dec 19, 2024
Merged via the queue into dev with commit 36c3af4 Dec 19, 2024
23 of 29 checks passed
@kemuru kemuru deleted the feat/ui-improvements branch December 19, 2024 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants