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

BA-2250: Limit group name length #217

Merged
merged 2 commits into from
Feb 17, 2025

Conversation

Ronan-Fernandes
Copy link
Contributor

@Ronan-Fernandes Ronan-Fernandes commented Feb 11, 2025

Acceptance Criteria

  • The group name should not affect the legibility of other components.
  • The Chat List card size should be uniform for all chats, the group name should not make the component become bigger
  • Limit the amount of characters shown from the Group Name in the Group Details and Chat List card to 20.
    • Implement a 3 dot (...) to signal that the name is bigger.

Current behavior
Loom: https://www.loom.com/share/3a11162195dd4f50b77b0b31506e0282?sid=58ea9c05-108f-47c8-9e71-d04aec5bd62d

Summary by CodeRabbit

  • Style

    • Enhanced text presentation across chat and group views so that long titles now display with an ellipsis, preventing unwanted wrapping and overflow.
  • New Features

    • Added a 20-character limit for group title inputs, ensuring consistent and constrained title lengths during creation and editing.

Copy link

coderabbitai bot commented Feb 11, 2025

Walkthrough

This update introduces a new TypographyWithEllipsis component to replace the standard Typography component across several files, enhancing text overflow handling for titles. The validation schema for group titles is updated to enforce a maximum length of 20 characters in both group creation and editing contexts. Additionally, input handling for text fields now includes a restriction on character count. Overall, the changes focus on improving the display and validation of titles without altering any core business logic or public API declarations.

Changes

File(s) Summary
packages/components/modules/messages/web/ChatRoomsList/ChatRoomItem/index.tsx
packages/components/modules/messages/web/GroupDetails/index.tsx
packages/components/modules/messages/web/ChatRoom/ChatRoomHeader/index.tsx
Replaced Typography components with TypographyWithEllipsis for improved text overflow handling in titles.
packages/components/modules/messages/web/CreateGroup/constants.ts
packages/components/modules/messages/web/EditGroup/constants.ts
packages/components/modules/messages/web/shared/EditGroupTitleAndImage/index.tsx
Updated validation schema for group titles to enforce a maximum length of 20 characters and modified input handling in EditGroupTitleAndImage.

Possibly related PRs

  • BA-1584 Add Storybook for Notifications #128: The changes in the main PR, which involve replacing the Typography component with TypographyWithEllipsis in the ChatRoomItem and GroupHeaderContainer, are related to the modifications in the ChatRoomHeader component of the retrieved PR, where the same replacement occurs.
  • BA-1964: update unread badge and divider behavior #159: The changes in the main PR, which involve replacing the Typography component with TypographyWithEllipsis in the ChatRoomItem and GroupDetails components, are related to the modifications in the retrieved PR that also replace Typography with TypographyWithEllipsis in the ChatRoomHeader, indicating a consistent update across components for text overflow handling.
  • BA-1904 Display Chat Header #148: The changes in the main PR, specifically the replacement of the Typography component with TypographyWithEllipsis in the ChatRoomItem component, are related to the modifications in the ChatRoomHeader component from the retrieved PR, which also replaces Typography with TypographyWithEllipsis for displaying the chat room title.

Suggested reviewers

  • anicioalexandre
  • deboracosilveira
  • priscilladeroode

Poem

I'm a little rabbit hopping in the code,
Trimming text lines on every node.
With noWrap and ellipsis so neat,
Every title fits in its seat.
Bugs are chased, and joy's bestowed –
CodeRabbit smiles along the road! 🐰


📜 Recent review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 0bc1407 and 7292a95.

📒 Files selected for processing (2)
  • packages/components/CHANGELOG.md (1 hunks)
  • packages/components/package.json (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • packages/components/package.json
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Component Test Packages
  • GitHub Check: Build and Lint Packages
🔇 Additional comments (1)
packages/components/CHANGELOG.md (1)

3-9: Changelog Entry for Version 1.0.8 - Group Name Length Limitation

The new patch release entry documents the changes to limit the group name length and use ellipsis for text overflow. This aligns well with the PR objective of ensuring that group names are truncated to a maximum of 20 characters to maintain layout consistency and readability. Please ensure that the corresponding implementation details (e.g., usage of the new TypographyWithEllipsis component and updated validation schemas) are clearly referenced elsewhere in the documentation or code comments.


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?

❤️ Share
🪧 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 generate docstrings to generate docstrings for this PR. (Beta)
  • @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 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.

@Ronan-Fernandes Ronan-Fernandes changed the title Limit group name length BA-2250: Limit group name length Feb 11, 2025
Copy link

@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: 0

🧹 Nitpick comments (1)
packages/components/modules/messages/web/__shared__/EditGroupTitleAndImage/index.tsx (1)

65-75: Consider adding a character counter for better UX.

While the maxLength constraint is correctly implemented, adding a character counter would provide better visual feedback to users about the remaining available characters.

 <TextField
   label="Group Name"
   inputProps={{ maxLength: 20 }}
+  helperText={`${watch(FORM_VALUE.title)?.length || 0}/20 characters`}
   control={control}
   disabled={isMutationInFlight}
   name={FORM_VALUE.title}
   className="h-[min-content]"
   onKeyUp={() => {
     trigger(FORM_VALUE.title)
   }}
 />
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between d78a123 and f3b0252.

📒 Files selected for processing (5)
  • packages/components/modules/messages/web/ChatRoomsList/ChatRoomItem/index.tsx (1 hunks)
  • packages/components/modules/messages/web/CreateGroup/constants.ts (1 hunks)
  • packages/components/modules/messages/web/EditGroup/constants.ts (1 hunks)
  • packages/components/modules/messages/web/GroupDetails/index.tsx (1 hunks)
  • packages/components/modules/messages/web/__shared__/EditGroupTitleAndImage/index.tsx (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
packages/components/modules/messages/web/CreateGroup/constants.ts (1)

20-23: LGTM! Clear and consistent validation.

The validation schema correctly implements the 20-character limit for group names with a clear error message.

packages/components/modules/messages/web/EditGroup/constants.ts (1)

25-28: LGTM! Consistent validation with group creation.

The validation schema matches the CreateGroup implementation, ensuring consistent behavior across the application.

packages/components/modules/messages/web/ChatRoomsList/ChatRoomItem/index.tsx (1)

136-142: LGTM! Proper text truncation implementation.

The Typography component correctly implements text truncation with ellipsis, ensuring consistent card sizes regardless of title length.

packages/components/modules/messages/web/GroupDetails/index.tsx (1)

149-154: LGTM! The text truncation implementation looks good.

The Typography component has been correctly configured to handle long group names by:

  • Using noWrap to prevent text from wrapping to multiple lines
  • Implementing text truncation with ellipsis using proper overflow properties

This implementation aligns well with Material-UI best practices for text truncation and maintains consistent UI layout as per the PR objectives.

@Ronan-Fernandes Ronan-Fernandes self-assigned this Feb 11, 2025
tsl-ps2
tsl-ps2 previously approved these changes Feb 13, 2025
@tsl-ps2 tsl-ps2 dismissed their stale review February 13, 2025 10:02

Want to update it

tsl-ps2
tsl-ps2 previously approved these changes Feb 13, 2025
Copy link
Collaborator

@tsl-ps2 tsl-ps2 left a comment

Choose a reason for hiding this comment

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

Left a small comment below.

@Ronan-Fernandes Ronan-Fernandes force-pushed the BA-2250-fe-limit-group-name-length branch from f3b0252 to 208d7b6 Compare February 13, 2025 20:33
Copy link

@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: 1

🧹 Nitpick comments (11)
packages/components/.scripts/build.mjs (1)

6-49: Consider optional concurrency management or partial success handling.
The build steps are well-organized and run in parallel where possible, but if one process fails, the entire build fails immediately. Depending on your use case, you might want finer-grained error handling or concurrency limits to allow partial successes or fallback flows.

packages/design-system/.scripts/build.mjs (1)

9-61: Consider adding error type information.

The error handling is good, but TypeScript type information for the error object would improve type safety.

-  } catch (error) {
+  } catch (error: unknown) {
-    if (error.signal !== 'SIGTERM') {
+    if (error && typeof error === 'object' && 'signal' in error && error.signal !== 'SIGTERM') {
packages/design-system/.scripts/dev-command.mjs (2)

18-21: Consider using TypeScript for better type safety.

The state variables would benefit from explicit type declarations.

+type BuildState = {
+  isBuilding: boolean;
+  needsRebuild: boolean;
+  buildTimeout: NodeJS.Timeout | null;
+  currentBuildProcesses: ExecaChildProcess[];
+};

-let isBuilding = false
-let needsRebuild = false
-let buildTimeout = null
-let currentBuildProcesses = []
+const buildState: BuildState = {
+  isBuilding: false,
+  needsRebuild: false,
+  buildTimeout: null,
+  currentBuildProcesses: [],
+};

70-85: Improve code readability with early returns.

The nested conditions in the ignored function can be simplified.

-  ignored: (filePath, stats) => {
-    if (
-      filePath.includes('node_modules') ||
-      filePath.includes('dist') ||
-      filePath.includes('tmp')
-    ) {
-      return true
-    }
-    if (stats && stats.isFile()) {
-      const relative = path.relative(rootDir, filePath).replace(/\\/g, '/')
-      if (!watchRegex.test(relative)) {
-        return true
-      }
-    }
-    return false
-  },
+  ignored: (filePath, stats) => {
+    // Ignore common directories
+    if (filePath.includes('node_modules') || filePath.includes('dist') || filePath.includes('tmp')) {
+      return true;
+    }
+    
+    // Only check regex for files
+    if (!stats?.isFile()) {
+      return false;
+    }
+    
+    const relative = path.relative(rootDir, filePath).replace(/\\/g, '/');
+    return !watchRegex.test(relative);
+  },
🧰 Tools
🪛 Biome (1.9.4)

[error] 78-78: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

turbo.json (1)

5-9: Document the required environment variable in README.

The BASEAPP_FRONTEND_TEMPLATE_PATH environment variable is now required for the dev task. This should be documented in the project's README to help developers set up their environment correctly.

Consider adding a section in the README:

## Development Setup

### Required Environment Variables

- `BASEAPP_FRONTEND_TEMPLATE_PATH`: Path to the baseapp frontend template
  - Example: `/path/to/baseapp-frontend-template`
  - Required for: Development tasks (`pnpm dev`)
.scripts/command-utils.mjs (2)

7-25: Consider adding timeout to process termination.

The process termination in cancelCurrentBuild could hang if a process doesn't respond to the kill signal.

 export const cancelCurrentBuild = async ({
   currentBuildProcesses,
   commandTag,
 }) => {
   console.log(chalk.red(`${commandTag} Canceling current build processes...`))
   for (const proc of currentBuildProcesses) {
     try {
-      proc.kill()
+      const killed = proc.kill()
+      if (!killed) {
+        console.warn(
+          chalk.yellow(
+            `${commandTag} Process ${proc.pid} did not respond to kill signal`
+          )
+        )
+      }
     } catch (err) {
       console.error(
         chalk.red(

76-96: Add timeout to file watcher.

The waitForReadyFile function could potentially wait indefinitely. Consider adding a timeout to prevent hanging.

 export const waitForReadyFile = async ({ readyFilePath, commandTag }) => {
   console.log(
     chalk.yellow(`${commandTag} Waiting for other packages to start...`)
   )
   try {
     await fs.access(readyFilePath)
     return
   } catch (err) {}
 
   return new Promise((resolve, reject) => {
+    const timeout = setTimeout(() => {
+      watcher.close()
+      reject(new Error('Timeout waiting for ready file'))
+    }, 60000) // 1 minute timeout
     const watcher = chokidar.watch(readyFilePath, { ignoreInitial: true })
     watcher.on("add", () => {
+      clearTimeout(timeout)
       watcher.close()
       resolve()
     })
     watcher.on("error", (err) => {
+      clearTimeout(timeout)
       watcher.close()
       reject(err)
     })
   })
 }
packages/components/.scripts/dev-command.mjs (3)

19-22: Consider using a state machine for build status.

The current state management using multiple variables could lead to race conditions. Consider using a state machine to make state transitions more explicit and safer.

-let isBuilding = false
-let needsRebuild = false
-let buildTimeout = null
-let currentBuildProcesses = []
+const BuildState = {
+  IDLE: 'idle',
+  BUILDING: 'building',
+  NEEDS_REBUILD: 'needs_rebuild'
+}
+
+const state = {
+  current: BuildState.IDLE,
+  buildTimeout: null,
+  currentBuildProcesses: [],
+  transition(to) {
+    console.log(`${commandTag} Build state: ${this.current} -> ${to}`)
+    this.current = to
+  }
+}

75-90: Improve file path filtering logic.

The file path filtering logic could be more maintainable using a configuration object.

+const IGNORED_PATHS = ['node_modules', 'dist', 'tmp']
+const WATCH_PATHS = {
+  common: true,
+  web: true,
+  native: true
+}
+
 ignored: (filePath, stats) => {
-    if (
-      filePath.includes('node_modules') ||
-      filePath.includes('dist') ||
-      filePath.includes('tmp')
-    ) {
+    if (IGNORED_PATHS.some(path => filePath.includes(path))) {
       return true
     }
     if (stats && stats.isFile()) {
       const relative = path.relative(rootDir, filePath).replace(/\\/g, '/')
-      if (!watchRegex.test(relative)) {
+      const [, type] = relative.match(/^modules\/(?:.*\/)?(common|web|native)\/.*$/) || []
+      if (!type || !WATCH_PATHS[type]) {
         return true
       }
     }
     return false
   },
🧰 Tools
🪛 Biome (1.9.4)

[error] 83-83: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


93-98: Consider using a more robust debouncing mechanism.

The current setTimeout-based debouncing could be improved using a proper debounce utility.

+const debounce = (fn, ms) => {
+  let timeoutId
+  return (...args) => {
+    clearTimeout(timeoutId)
+    timeoutId = setTimeout(() => fn(...args), ms)
+  }
+}
+
+const debouncedBuild = debounce(runWatchBuild, 2000)
+
 watcher.on('all', (event, changedPath) => {
   const relativePath = path.relative(rootDir, changedPath).replace(/\\/g, '/')
   console.log(`${commandTag} Detected event "${event}" on: ${relativePath}`)
-  if (buildTimeout) clearTimeout(buildTimeout)
-  buildTimeout = setTimeout(runWatchBuild, 2000)
+  debouncedBuild()
 })
README.md (1)

91-94: Consider adding a comma for better readability.

-Our development mode is designed to provide immediate feedback as you work on our monorepo apps and packages. In dev mode, each package automatically watches for changes in its source files, rebuilds itself using a custom build process, and synchronizes its output (bundled code, type declarations, etc.) to the consumer app.
+Our development mode is designed to provide immediate feedback as you work on our monorepo apps and packages. In dev mode, each package automatically watches for changes in its source files, rebuilds itself using a custom build process, and synchronizes its output (bundled code, type declarations, etc.), to the consumer app.
🧰 Tools
🪛 LanguageTool

[uncategorized] ~93-~93: Possible missing comma found.
Context: ...ke are quickly reflected in the running application without the need to manually rebuild or...

(AI_HYDRA_LEO_MISSING_COMMA)

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between f3b0252 and 208d7b6.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (22)
  • .scripts/command-utils.mjs (1 hunks)
  • README.md (1 hunks)
  • package.json (1 hunks)
  • packages/components/.scripts/build-command.mjs (1 hunks)
  • packages/components/.scripts/build.mjs (1 hunks)
  • packages/components/.scripts/dev-command.mjs (1 hunks)
  • packages/components/README.md (1 hunks)
  • packages/components/modules/messages/web/ChatRoomsList/ChatRoomItem/index.tsx (1 hunks)
  • packages/components/modules/messages/web/CreateGroup/constants.ts (1 hunks)
  • packages/components/modules/messages/web/EditGroup/constants.ts (1 hunks)
  • packages/components/modules/messages/web/GroupDetails/index.tsx (1 hunks)
  • packages/components/modules/messages/web/__shared__/EditGroupTitleAndImage/index.tsx (1 hunks)
  • packages/components/package.json (3 hunks)
  • packages/components/tsup.config.ts (1 hunks)
  • packages/design-system/.scripts/build-command.mjs (1 hunks)
  • packages/design-system/.scripts/build.mjs (1 hunks)
  • packages/design-system/.scripts/dev-command.mjs (1 hunks)
  • packages/design-system/README.md (1 hunks)
  • packages/design-system/package.json (2 hunks)
  • packages/design-system/tsup.config.ts (1 hunks)
  • pnpm-workspace.yaml (2 hunks)
  • turbo.json (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • packages/design-system/README.md
🚧 Files skipped from review as they are similar to previous changes (5)
  • packages/components/modules/messages/web/shared/EditGroupTitleAndImage/index.tsx
  • packages/components/modules/messages/web/CreateGroup/constants.ts
  • packages/components/modules/messages/web/EditGroup/constants.ts
  • packages/components/modules/messages/web/GroupDetails/index.tsx
  • packages/components/modules/messages/web/ChatRoomsList/ChatRoomItem/index.tsx
🧰 Additional context used
🪛 LanguageTool
README.md

[uncategorized] ~93-~93: Possible missing comma found.
Context: ...ke are quickly reflected in the running application without the need to manually rebuild or...

(AI_HYDRA_LEO_MISSING_COMMA)


[misspelling] ~97-~97: This word is normally spelled as one.
Context: ...@baseapp-frontend/design-system`—have a multi-step build process. When you run: ```bash p...

(EN_COMPOUNDS_MULTI_STEP)

🪛 Biome (1.9.4)
packages/components/.scripts/dev-command.mjs

[error] 83-83: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

packages/design-system/.scripts/dev-command.mjs

[error] 78-78: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Build and Lint Packages
🔇 Additional comments (19)
packages/components/.scripts/build.mjs (1)

1-2: Imports are appropriate and concise.
No issues found with using chalk and execa here.

packages/components/.scripts/build-command.mjs (1)

1-3: Entry point script looks good.
The script imports and invokes runBuild immediately without further logic, which is a clear and straightforward approach. No issues found.

packages/design-system/.scripts/build-command.mjs (1)

1-3: Straightforward build command invocation.
Similar to the components build command, this script cleanly triggers runBuild. No issues found.

packages/components/tsup.config.ts (1)

7-7: LGTM! Clean build output directory.

Setting clean: true ensures a fresh build by removing old artifacts. This is a good practice to prevent stale files from affecting the build output.

packages/design-system/tsup.config.ts (1)

7-7: LGTM! Clean build output directory.

Setting clean: true ensures a fresh build by removing old artifacts. This is a good practice to prevent stale files from affecting the build output.

packages/design-system/.scripts/build.mjs (1)

1-8: LGTM! Well-organized imports and constants.

The imports are well-structured, and the command tag constant provides consistent logging identification.

packages/design-system/.scripts/dev-command.mjs (1)

88-93: LGTM! Good use of debouncing.

The debouncing implementation with setTimeout prevents rapid consecutive builds, which is a good practice.

package.json (1)

26-30:

✅ Verification successful

LGTM! Dependencies are well-chosen for build tooling.

The new dependencies align well with the project's needs:

  • chalk for colorized console output
  • chokidar for efficient file watching
  • execa for improved process execution

Let's verify these are the latest stable versions:


🌐 Web query:

What are the latest stable versions of chalk, chokidar, and execa npm packages?

💡 Result:

The latest stable versions of the specified npm packages as of February 2025 are:

Chalk

  • v5.4.1
    Released December 21, 2024[1][8].
    Major changes since v4:
    • Pure ESM package requiring Node.js ≥12[1][9]
    • Reduced dependencies (now dependency-free)[1]
    • Dropped .keyword(), .hsl(), and other color methods[1]

Chokidar

  • v4.x (Latest: 4.x)
    Released December 18, 2024[2][10].
    Key updates:
    • Hybrid CommonJS/ESM support[2]
    • Requires Node.js ≥14[2]
    • Simplified dependencies (from 13 → 1)[2]

Execa

  • v9.x (Latest: v9.x)
    Latest release includes:
    • IPC message improvements between processes[3]
    • gracefulCancel option for subprocess termination[3]

For production environments:

npm install chalk@5 chokidar@4 execa@9

Citations:


LGTM! Dependencies are well-chosen and their versions are confirmed as the latest stable releases as of February 2025.

  • The package.json correctly specifies:
    • chalk at "^5.4.1", which matches the latest stable release.
    • chokidar at "^4.0.3", aligning with the latest 4.x series.
    • execa at "^9.5.2", consistent with the current major version.
  • The use of caret (^) for version ranges ensures minor and patch updates are automatically incorporated.
pnpm-workspace.yaml (2)

146-149: LGTM! Well-chosen development tools.

The new dev-tools catalog includes essential utilities that enhance the development workflow:

  • chalk: For styled console output
  • chokidar: For efficient file watching
  • execa: For reliable process execution

33-46:

❓ Verification inconclusive

Verify the impact of removing react-native-core catalog.

The removal of the react-native-core catalog suggests a shift away from mobile development. Please ensure that:

  1. All mobile-specific features have been properly migrated or deprecated.
  2. No existing components depend on the removed packages.

Run the following script to check for any remaining dependencies on the removed packages:


🏁 Script executed:

#!/bin/bash
# Description: Check for any remaining dependencies on removed react-native packages.

# Search for imports of removed packages
rg -l '@gorhom/bottom-sheet|@react-navigation/drawer|@react-navigation/native|expo-|react-native'

# Search for package.json files that might still reference these packages
fd 'package.json' -x jq -r '.dependencies,.devDependencies | select(. != null) | keys[]' {} | grep -E 'bottom-sheet|react-navigation|expo-|react-native'

Length of output: 7583


Verify the impact of the react-native-core catalog removal.

The removal of the react-native-core catalog from pnpm-workspace.yaml is noted; however, our search shows that many mobile-specific packages (such as react-native, expo-constants, expo-secure-store, @gorhom/bottom-sheet, among others) continue to be referenced across the codebase—including in mocks, test configurations, and source files. Please verify that:

  • Mobile-specific features in the app are either fully migrated or intentionally maintained via a new dependency management approach.
  • All mocks, tests, and configurations referencing these packages are updated if mobile development is being deprecated.
  • The new dev-tools catalog does not conflict with or inadvertently affect mobile package resolution.
packages/design-system/package.json (1)

11-12: LGTM! Build system improvements.

The changes enhance the build and development workflow:

  1. Scripts now use dedicated command files for better organization
  2. Dev dependencies correctly reference the new dev-tools catalog

Also applies to: 116-118, 121-121

packages/components/package.json (3)

7-7: LGTM! More descriptive script name.

Renaming from babel:bundle to babel:transpile better reflects the script's purpose, as it's specifically handling the transpilation step.


12-13: Verify the version constraint change.

The dependency version constraint for @baseapp-frontend/config has been changed from workspace:* to workspace:^. Please ensure this change doesn't affect package resolution or cause version conflicts.

Also applies to: 115-115


147-149: LGTM! Development tooling improvements.

The addition of dev-tools dependencies enhances the development workflow with file watching, process execution, and console output styling capabilities.

Also applies to: 156-156

README.md (2)

86-86: LGTM! More precise package filter command.

Using the full package name @baseapp-frontend/authentication is more explicit and consistent with the package naming convention.


97-124: LGTM! Excellent documentation of the build process.

The detailed explanation of the build steps and example output logs greatly improves the developer experience by setting clear expectations about the build process.

🧰 Tools
🪛 LanguageTool

[misspelling] ~97-~97: This word is normally spelled as one.
Context: ...@baseapp-frontend/design-system`—have a multi-step build process. When you run: ```bash p...

(EN_COMPOUNDS_MULTI_STEP)

packages/components/README.md (3)

21-21: LGTM! Clear and informative package description.

The description effectively communicates the package contents and introduces Storybook's purpose.


26-26: LGTM! More precise package filtering.

The updated command using the full package name @baseapp-frontend/components is more precise and better suited for monorepo environments.


21-26: Verify if these documentation changes belong in this PR.

While the documentation improvements are valuable, they appear unrelated to the PR's objective of limiting group name length in the UI. Consider submitting these changes in a separate documentation-focused PR.

@Ronan-Fernandes Ronan-Fernandes force-pushed the BA-2250-fe-limit-group-name-length branch from 208d7b6 to 7ddbdd4 Compare February 13, 2025 20:37
Copy link

changeset-bot bot commented Feb 13, 2025

⚠️ No Changeset found

Latest commit: 7292a95

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

@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: 0

🧹 Nitpick comments (2)
packages/components/modules/messages/web/ChatRoom/ChatRoomHeader/index.tsx (2)

82-93: LGTM! The implementation effectively handles text overflow.

The Typography component's configuration successfully addresses the PR objectives by preventing text overflow and maintaining consistent component sizes. The responsive maxWidth values ensure proper display across different screen sizes.

Consider improving the inline comment to be more specific about which parent divs are being referenced. For example:

-maxWidth: isUpToMd ? '300px' : '200px', // Prevent text from overflowing since none of the divs has a good max width
+maxWidth: isUpToMd ? '300px' : '200px', // Prevent text overflow in ChatTitleContainer and Box wrappers

91-91: Consider extracting maxWidth values as constants.

To improve maintainability and reusability, consider extracting the maxWidth values into named constants at the top of the file.

+const TITLE_MAX_WIDTH = {
+  DESKTOP: '300px',
+  MOBILE: '200px',
+} as const;

 // ...

-maxWidth: isUpToMd ? '300px' : '200px',
+maxWidth: isUpToMd ? TITLE_MAX_WIDTH.DESKTOP : TITLE_MAX_WIDTH.MOBILE,
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 208d7b6 and 7ddbdd4.

📒 Files selected for processing (6)
  • packages/components/modules/messages/web/ChatRoom/ChatRoomHeader/index.tsx (1 hunks)
  • packages/components/modules/messages/web/ChatRoomsList/ChatRoomItem/index.tsx (1 hunks)
  • packages/components/modules/messages/web/CreateGroup/constants.ts (1 hunks)
  • packages/components/modules/messages/web/EditGroup/constants.ts (1 hunks)
  • packages/components/modules/messages/web/GroupDetails/index.tsx (1 hunks)
  • packages/components/modules/messages/web/__shared__/EditGroupTitleAndImage/index.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • packages/components/modules/messages/web/CreateGroup/constants.ts
  • packages/components/modules/messages/web/shared/EditGroupTitleAndImage/index.tsx
  • packages/components/modules/messages/web/EditGroup/constants.ts
  • packages/components/modules/messages/web/ChatRoomsList/ChatRoomItem/index.tsx
  • packages/components/modules/messages/web/GroupDetails/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Build and Lint Packages

@Ronan-Fernandes Ronan-Fernandes changed the base branch from master to BA-2259-improve-dev-mode February 14, 2025 11:51
@Ronan-Fernandes Ronan-Fernandes force-pushed the BA-2250-fe-limit-group-name-length branch from 7ddbdd4 to be7d63d Compare February 14, 2025 14:11
@anicioalexandre anicioalexandre force-pushed the BA-2259-improve-dev-mode branch 2 times, most recently from f25e030 to e183314 Compare February 17, 2025 14:01
Base automatically changed from BA-2259-improve-dev-mode to master February 17, 2025 14:43
@anicioalexandre anicioalexandre dismissed tsl-ps2’s stale review February 17, 2025 14:43

The base branch was changed.

@Ronan-Fernandes Ronan-Fernandes force-pushed the BA-2250-fe-limit-group-name-length branch 2 times, most recently from 5633777 to ea11f32 Compare February 17, 2025 15:11
@Ronan-Fernandes Ronan-Fernandes force-pushed the BA-2250-fe-limit-group-name-length branch from ea11f32 to 0bc1407 Compare February 17, 2025 18:40
@anicioalexandre anicioalexandre merged commit f207740 into master Feb 17, 2025
8 checks passed
@anicioalexandre anicioalexandre deleted the BA-2250-fe-limit-group-name-length branch February 17, 2025 20:02
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.

3 participants