Default social input send behavior #199
Conversation
Ronan-Fernandes
requested review from
anicioalexandre,
deboracosilveira and
priscilladeroode
as code owners
|
WalkthroughThe changes introduce a new optional property, Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant SI as SocialInput Component
participant D as DefaultKeyDown Logic
U->>+SI: Keydown event (e.g., "Enter" or CTRL+Enter)
SI-->>SI: Check for custom onKeyDown prop
alt Custom Handler Provided
SI->>Custom: Execute provided onKeyDown(event)
else No Custom Handler
SI->>+D: Call defaultKeyDown(event)
D-->>-SI: Process event (CTRL+Enter triggers submission)
end
SI-->>-U: Return event feedback/result
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
packages/components/modules/__shared__/web/SocialInput/index.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/components/.eslintrc.js
packages/components/schema.graphqlOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/components/.eslintrc.js
packages/components/modules/__shared__/web/SocialInput/types.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/components/.eslintrc.js
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (2)
packages/components/modules/__shared__/SocialInput/index.tsx (1)
97-97: Consider adding error boundaries around form submission.While the implementation is correct, consider wrapping the form submission in a try-catch block to handle potential errors gracefully.
const defaultKeyDown: KeyboardEventHandler<HTMLDivElement> = (event) => { if (event.key === 'Enter' && event.ctrlKey) { event.preventDefault() - form.handleSubmit(submit)(event) + try { + form.handleSubmit(submit)(event) + } catch (error) { + console.error('Failed to submit form:', error) + } } }packages/components/modules/__shared__/SocialInput/__storybook__/SocialInput.mdx (1)
33-33: Enhance prop documentation with more details.While the documentation mentions the default behavior, it would be helpful to explicitly state:
- The default behavior uses CTRL+Enter to send messages
- Enter alone creates a new line
- How to override the default behavior
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
packages/components/__generated__/ActivityLogsFragment.graphql.tsis excluded by!**/__generated__/**packages/components/__generated__/ActivityLogsPaginationQuery.graphql.tsis excluded by!**/__generated__/**packages/components/__generated__/ActivityLogsQuery.graphql.tsis excluded by!**/__generated__/**
📒 Files selected for processing (3)
packages/components/modules/__shared__/SocialInput/__storybook__/SocialInput.mdx(2 hunks)packages/components/modules/__shared__/SocialInput/index.tsx(3 hunks)packages/components/modules/__shared__/SocialInput/types.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
packages/components/modules/__shared__/SocialInput/types.ts (1)
18-18: LGTM! Well-typed optional prop addition.The
onKeyDownprop is correctly typed as an optional handler for keyboard events, maintaining backward compatibility while providing type safety.packages/components/modules/__shared__/SocialInput/index.tsx (1)
82-86: LGTM! Clean implementation of CTRL+Enter behavior.The
defaultKeyDownhandler correctly implements the new message sending behavior:
- Checks for both Enter key and CTRL modifier
- Prevents default event to avoid unwanted line breaks
- Properly triggers form submission
packages/components/modules/__shared__/SocialInput/__storybook__/SocialInput.mdx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Would it make sense to expose form data or at least the submit method to the onKeyDown handler? Suppose I want to customize the behavior on the template and switch back to sending messages when pressing enter. How would I do that? With the current code, if I try to write an onKeyDown handler on the template, I only get access to the event, but not the current form or any other social input variables. So I cannot really do anything except for stopping the event from propagating, right? (Triggering some other event or changing the event variables so that this new event does what I want sounds like a really bad hack.)
Ronan-Fernandes
force-pushed
the
BA-2197-fe-social-input-heydown-behavior
branch
from
17b049b to
6346d75
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/components/modules/__shared__/web/SocialInput/index.tsx (1)
82-87: Add JSDoc for the defaultKeyDown function.The function implementation is correct, but adding documentation would improve maintainability.
+ /** + * Default keyboard event handler that submits the form when CTRL + Enter is pressed. + * @param event - The keyboard event + */ const defaultKeyDown: KeyboardEventHandler<HTMLDivElement> = (event) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/components/modules/__shared__/web/SocialInput/__storybook__/SocialInput.mdx(2 hunks)packages/components/modules/__shared__/web/SocialInput/index.tsx(3 hunks)packages/components/modules/__shared__/web/SocialInput/types.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and Lint Packages
🔇 Additional comments (3)
packages/components/modules/__shared__/web/SocialInput/types.ts (1)
1-1: LGTM! Type definitions are accurate.The new
onKeyDownprop is correctly typed as an optional keyboard event handler for HTMLDivElement, which aligns with the PR's objective of enabling custom keyboard event handling.Also applies to: 18-18
packages/components/modules/__shared__/web/SocialInput/index.tsx (1)
71-71: LGTM! Implementation aligns with PR objectives.The keyboard event handling is well-implemented:
- Custom handlers can be provided via the
onKeyDownprop- Default behavior (CTRL + Enter to send) is provided as a fallback
Also applies to: 97-97
packages/components/modules/__shared__/web/SocialInput/__storybook__/SocialInput.mdx (1)
33-33: LGTM! Documentation is clear and accurate.The prop documentation clearly describes the default behavior of sending with CTRL + Enter.
packages/components/modules/__shared__/web/SocialInput/__storybook__/SocialInput.mdx
Outdated
Show resolved
Hide resolved
Ronan-Fernandes
force-pushed
the
BA-2197-fe-social-input-heydown-behavior
branch
from
6346d75 to
00b204f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/components/modules/__shared__/web/SocialInput/__storybook__/SocialInput.mdx(2 hunks)packages/components/modules/__shared__/web/SocialInput/index.tsx(3 hunks)packages/components/modules/__shared__/web/SocialInput/types.ts(2 hunks)packages/components/schema.graphql(70 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/components/schema.graphql
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/components/modules/shared/web/SocialInput/storybook/SocialInput.mdx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and Lint Packages
🔇 Additional comments (2)
packages/components/modules/__shared__/web/SocialInput/types.ts (1)
1-1: LGTM! Well-typed keyboard event handler.The new
onKeyDownprop is correctly typed withKeyboardEvent<HTMLDivElement>and a submission callback, providing flexibility for custom keyboard event handling.Also applies to: 18-18
packages/components/modules/__shared__/web/SocialInput/index.tsx (1)
82-87: LGTM! Correct implementation of CTRL + Enter submission.The
defaultKeyDownfunction correctly implements the PR objectives by:
- Triggering submission only when CTRL + Enter is pressed
- Preventing default behavior to avoid unwanted line breaks
packages/components/modules/__shared__/web/SocialInput/index.tsx
Outdated
Show resolved
Hide resolved
packages/components/modules/__shared__/web/SocialInput/index.tsx
Outdated
Show resolved
Hide resolved
Ronan-Fernandes
force-pushed
the
BA-2197-fe-social-input-heydown-behavior
branch
from
00b204f to
7a2b017
Compare
Ronan-Fernandes
force-pushed
the
BA-2197-fe-social-input-heydown-behavior
branch
from
7a2b017 to
ef5468e
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/components/modules/__shared__/web/SocialInput/__storybook__/SocialInput.mdx (1)
67-72:⚠️ Potential issueUpdate example to match the component's default behavior.
The example shows preventing default on Enter without Shift, which contradicts the component's default behavior of using CTRL + Enter to send.
Apply this diff to align with the default behavior:
- onKeyDown={(event, onSubmit) => { - if (event.key === 'Enter' && !event.shiftKey) { - event.preventDefault() - onSubmit() - } - }} + onKeyDown={(event, onSubmit) => { + if (event.key === 'Enter' && event.ctrlKey) { + event.preventDefault() + onSubmit() + } + }}
🧹 Nitpick comments (1)
packages/components/modules/__shared__/web/SocialInput/__storybook__/SocialInput.mdx (1)
33-33: Enhance onKeyDown prop documentation.While the documentation mentions the default behavior, it would be helpful to add more details about:
- The event and onSubmit callback parameters
- How to prevent default behavior
- Common use cases
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/components/CHANGELOG.md(1 hunks)packages/components/modules/__shared__/web/SocialInput/__storybook__/SocialInput.mdx(2 hunks)packages/components/modules/__shared__/web/SocialInput/index.tsx(3 hunks)packages/components/modules/__shared__/web/SocialInput/types.ts(2 hunks)packages/components/package.json(1 hunks)packages/components/schema.graphql(70 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/components/package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/components/modules/shared/web/SocialInput/types.ts
- packages/components/schema.graphql
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and Lint Packages
🔇 Additional comments (4)
packages/components/modules/__shared__/web/SocialInput/index.tsx (3)
71-71: LGTM! Clean implementation of the keyboard event handling.The implementation of
defaultKeyDownwith CTRL+Enter behavior is clean and aligns well with the PR objectives.Also applies to: 82-87
89-95: LGTM! Proper handling of custom and default keyboard events.The implementation correctly prevents duplicate submissions by using an else block, as suggested in the past review comments.
105-105: LGTM! Proper prop passing to SocialTextField.The
handleKeyDownfunction is correctly passed down to the SocialTextField component.packages/components/CHANGELOG.md (1)
3-8: LGTM! Clear and accurate changelog entry.The changelog entry properly describes the changes and follows semantic versioning.
* Default social input send behavior * Versioning
* Default social input send behavior * Versioning
* Default social input send behavior * Versioning
* feat: delete chat room messages * BA-2197: default social input send behavior (#199) * Default social input send behavior * Versioning * fix: chat rooms refetch issue * fix: decrease border radius on system messages --------- Co-authored-by: Ronan <62265281+Ronan-Fernandes@users.noreply.github.com>
Acceptance Criteria
Customize the component so that future devs can have more freedom to create a custom behavior for sending the message using the keyboard.
Change the default behavior of the current component to be:
Given I am writing a message, when I hit the Enter Key, it should create a line break in the text where I can keep writing.
Given I am writing a message, when I hit CTRL + Enter, then it should send the message I have input.
Update storybook to include this new behavior.
Summary by CodeRabbit
New Features
Documentation
Style
Chores