-
Notifications
You must be signed in to change notification settings - Fork 200
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
style(components): change button state styling to fix disappearing buttons #2877
Conversation
- Update button classes to include hover and enabled states - Remove unused CSS rules to streamline styles (your buttons now have more states than a soap opera character)
|
WalkthroughThis pull request introduces several modifications across multiple components in the backoffice application, primarily focusing on enhancing the styling of Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
There was a problem hiding this 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
🧹 Outside diff range and nitpick comments (2)
apps/backoffice-v2/src/lib/blocks/components/CallToActionLegacy/CallToActionLegacy.tsx (1)
192-195
: LGTM! Good use of conditional classes for button statesThe implementation properly handles both the resetable and non-resetable states of the Re-upload button, with appropriate hover effects.
Consider extracting these common button state classes into a shared utility class or Tailwind plugin for consistency across the application:
-className={ctw({ - 'flex gap-2': isReuploadResetable, - 'enabled:bg-warning enabled:hover:bg-warning/90': !isReuploadResetable, -})} +className={ctw( + 'btn-base', // New utility class that includes common enabled/hover states + { + 'flex gap-2': isReuploadResetable, + 'btn-warning': !isReuploadResetable, // New utility class for warning variant + } +)}apps/backoffice-v2/src/lib/blocks/hooks/useDirectorsBlocks/useDirectorsBlocks.tsx (1)
256-256
: LGTM! Consider extracting common button styles.The added className properly implements the enabled state styling and hover effect, which aligns with the PR's goal of improving button visibility. The success color and hover opacity provide good visual feedback.
Consider extracting these common button styles into a shared constant or utility class, as similar styling might be needed for other buttons across the application. This would ensure consistency and make future updates easier.
// Example: Create a shared styles object const buttonStateStyles = { success: 'enabled:bg-success enabled:hover:bg-success/90', // Add other variants as needed } as const; // Usage className={buttonStateStyles.success}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
apps/backoffice-v2/src/domains/notes/Notes.tsx
(1 hunks)apps/backoffice-v2/src/index.css
(1 hunks)apps/backoffice-v2/src/lib/blocks/components/CallToActionLegacy/CallToActionLegacy.tsx
(2 hunks)apps/backoffice-v2/src/lib/blocks/components/EditableDetails/EditableDetails.tsx
(1 hunks)apps/backoffice-v2/src/lib/blocks/hooks/useDirectorsBlocks/useDirectorsBlocks.tsx
(1 hunks)apps/backoffice-v2/src/lib/blocks/hooks/useDocumentBlocks/useDocumentBlocks.tsx
(1 hunks)apps/backoffice-v2/src/pages/Entity/components/Case/actions-variants/DefaultActions/DefaultActions.tsx
(2 hunks)apps/backoffice-v2/src/pages/Root/Root.page.tsx
(1 hunks)apps/backoffice-v2/src/pages/SignIn/SignIn.page.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- apps/backoffice-v2/src/lib/blocks/components/EditableDetails/EditableDetails.tsx
- apps/backoffice-v2/src/pages/SignIn/SignIn.page.tsx
- apps/backoffice-v2/src/lib/blocks/hooks/useDocumentBlocks/useDocumentBlocks.tsx
🔇 Additional comments (6)
apps/backoffice-v2/src/pages/Root/Root.page.tsx (1)
36-40
: Verify chatbot positioning after iframe removal
The removal of the iframe wrapper might affect the chatbot's positioning and z-index stacking context. Please ensure that:
- The chatbot still appears above other UI elements
- The toggle functionality works correctly
- The chatbot maintains its expected position in the layout
✅ Verification successful
Chatbot positioning is handled by Botpress Webchat component
The removal of the iframe wrapper is safe because:
- The chatbot uses
@botpress/webchat
library components (Webchat
,WebchatProvider
) which handle their own positioning and z-index stacking context - No custom positioning styles were found in the codebase
- The toggle functionality is properly maintained through
isWebchatOpen
state andtoggleIsWebchatOpen
handler
The iframe removal simplifies the component structure without affecting the chatbot's functionality or positioning.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any z-index or positioning styles related to the chatbot
rg -g '*.{css,scss}' -A 2 'chatbot|webchat'
Length of output: 45
Script:
#!/bin/bash
# Let's check the Chatbot component implementation for any styling
fd -e tsx -e ts -e css -e scss "Chatbot" --exec cat {}
Length of output: 56
Script:
#!/bin/bash
# Search for the Chatbot component file using a broader search
fd "Chatbot" --type f
# Then let's check for any webchat or chatbot related styling in the entire codebase
rg -g '*.{tsx,ts,css,scss,styled.ts}' -i "(webchat|chatbot|\.webchat|\.chatbot)"
Length of output: 2714
apps/backoffice-v2/src/pages/Entity/components/Case/actions-variants/DefaultActions/DefaultActions.tsx (2)
38-42
: LGTM: Enhanced button visibility with proper state handling
The addition of enabled:bg-warning enabled:hover:bg-warning/90
classes improves button visibility and hover state feedback.
66-67
: LGTM: Consistent loading state handling
Good use of conditional styling to handle the enabled state based on the loading status.
apps/backoffice-v2/src/domains/notes/Notes.tsx (1)
91-91
: LGTM: Improved button styling with accessibility support
The updated button styling:
- Adds proper enabled state handling with hover effects
- Maintains accessibility with aria-disabled styles
- Follows the consistent pattern across other components
apps/backoffice-v2/src/lib/blocks/components/CallToActionLegacy/CallToActionLegacy.tsx (1)
261-261
: LGTM! Consistent button state styling
The enabled state styling matches the PR objectives and maintains consistency with other button implementations.
apps/backoffice-v2/src/lib/blocks/hooks/useDirectorsBlocks/useDirectorsBlocks.tsx (1)
Line range hint 252-259
: Verify button visibility across all states
The button combines motion animations (opacity: isApproveDisabled ? 0.5 : ...
) with enabled state styles. Let's ensure the button remains visible across all combinations of states.
✅ Verification successful
Button visibility pattern is consistent across the codebase
The codebase shows a consistent pattern for handling button visibility in disabled states:
-
All
MotionButton
instances use the same opacity animation pattern:- Default opacity from
motionButtonProps.animate.opacity
- Opacity 0.5 when disabled
- Default opacity from
-
The button's enabled state styling (
enabled:bg-success enabled:hover:bg-success/90
) follows the established pattern used across multiple components in the codebase. -
Disabled state handling is consistently implemented with:
disabled
prop for click preventionopacity: 0.5
for visual feedbackaria-disabled
attributes for accessibility
The implementation in the review matches these established patterns and correctly handles button visibility across all states.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for similar button patterns to ensure consistent visibility handling
# Check for other instances of MotionButton with opacity animations
ast-grep --pattern 'MotionButton {
$$$
animate={
$$$
opacity: $$$
}
$$$
}'
# Check for other buttons with enabled state styling
rg "enabled:bg-" --type tsx
Length of output: 148
Script:
#!/bin/bash
# Let's try different approaches to find similar patterns
# Search for MotionButton usage with opacity
rg -t ts -t tsx "MotionButton.*opacity"
# Search for enabled: class patterns (without file type restriction)
rg "enabled:(bg|hover)"
# Find all MotionButton component definitions and usages
rg -t ts -t tsx "MotionButton"
# Look for disabled state handling in buttons
rg -t ts -t tsx "disabled=\{.*\}"
Length of output: 1843
Script:
#!/bin/bash
# Let's search without file type restrictions since tsx isn't recognized
# Find MotionButton usages
rg "MotionButton" -A 5 -B 5
# Find disabled state handling with opacity
rg "disabled.*opacity|opacity.*disabled"
# Find aria-disabled styling patterns
rg "aria-disabled"
Length of output: 65830
apps/backoffice-v2/src/lib/blocks/components/CallToActionLegacy/CallToActionLegacy.tsx
Outdated
Show resolved
Hide resolved
...ffice-v2/src/pages/Entity/components/Case/actions-variants/DefaultActions/DefaultActions.tsx
Show resolved
Hide resolved
- Change button styles from slate to primary background and hover colors - Ensure consistency across different components for better UX (your color choices are so dark, they could be the backdrop for a horror movie)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job. 💪🏼
* feat(filters): implement create filter functionality (#2881) * style(components): change button state styling to fix disappearing buttons (#2877) * style(components): enhance button state styling for better UX - Update button classes to include hover and enabled states - Remove unused CSS rules to streamline styles (your buttons now have more states than a soap opera character) * fix(buttons): update background color for button components - Change button styles from slate to primary background and hover colors - Ensure consistency across different components for better UX (your color choices are so dark, they could be the backdrop for a horror movie) --------- Co-authored-by: Omri Levy <61207713+Omri-Levy@users.noreply.github.com> * fix(stepper): improve step display and clean up formatting - Refactor step display to enhance layout and readability - Add a new no-op constant to built-in events - Clean up transition validation logic (Your transition validation is so confusing, it makes find-and-replace look like a clear path) * Date input improvements (#2889) * refactor(*): changed handling of date inputs * ci(*): testing path change * temporarily disabled test * updated hook name --------- Co-authored-by: Tomer Shvadron <tomers@ballerine.com> * EditableDetailsV2 input type improvement (#2891) * refactor(*): changed handling of date inputs * ci(*): testing path change * temporarily disabled test * updated hook name * fix(backoffice-v2): no longer looking at form value for input type --------- Co-authored-by: Tomer Shvadron <tomers@ballerine.com> * EditableDetailsV2 added ability to override input type (#2892) * refactor(*): changed handling of date inputs * ci(*): testing path change * temporarily disabled test * updated hook name * fix(backoffice-v2): no longer looking at form value for input type * feat(backoffice-v2): added a way to override input type --------- Co-authored-by: Tomer Shvadron <tomers@ballerine.com> * Dev 318/action alert delivery (#2893) * feat: adding changes for sending alerts to specific channel * fix: added change in hotfix action --------- Co-authored-by: Alon Peretz <8467965+alonp99@users.noreply.github.com> Co-authored-by: Shane <66246046+shanegrouber@users.noreply.github.com> Co-authored-by: Tomer Shvadron <tomers@ballerine.com> Co-authored-by: Chirag <134513193+codechirag123@users.noreply.github.com>
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores