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

feat(UserGuide): update UserGuide component with new highlight functionality #1487

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

JoannaSikora
Copy link
Contributor

@JoannaSikora JoannaSikora commented Jan 16, 2025

Resolves: #1486

Description

Updated version of UserGuide:

  • UserGuideStep component - UI update with new designs
  • UserGuide component - UI update with new designs (floating element with animated arrow), updated the component API by removing unnecessary properties

I moved UserGuide to its own stories as it no longer relates to the Tooltip.
Added advanced example of implementation.

Storybook

https://feature-user-guide--613a8e945a5665003a05113b.chromatic.com/?path=/story/components-userguide--example

Checklist

Obligatory:

  • Self review (use this as your final check for proposed changes before requesting the review)
  • Add correct label
  • Assign pull request with the correct issue

Summary by CodeRabbit

Based on the comprehensive changes, here are the updated release notes:

  • Stylelint Configuration

    • Updated .stylelintrc.json configuration rules
  • Accessibility Improvements

    • Added id prop support to multiple components for better DOM targeting and accessibility
    • Enhanced component identification across various UI elements
  • User Guide Removal

    • Completely removed previous User Guide implementation
    • Deleted related components, stories, and documentation
  • New User Guide Implementation

    • Introduced a new User Guide component with enhanced functionality
    • Added documentation, styling, and interactive stories for the new implementation
  • Transition Utilities

    • Added new transition duration variable for moderate animations
  • Component Core Props

    • Extended core component properties to include optional id

Copy link
Contributor

coderabbitai bot commented Jan 16, 2025

Walkthrough

The pull request introduces a comprehensive overhaul of the User Guide component, removing the previous implementation and introducing a new, more sophisticated version with enhanced styling, positioning, and interaction capabilities. The changes span multiple files across the react-components package, focusing on creating a more dynamic and contextually aware user guidance system.

Changes

File Change Summary
.stylelintrc.json Added no-invalid-position-at-import-rule rule
packages/react-components/src/components/ActionBar/ActionBar.tsx Added id prop to Button component
packages/react-components/src/components/AppFrame/* Added id prop to various navigation components
packages/react-components/src/components/Tooltip/* Removed UserGuide-related components and documentation
packages/react-components/src/components/UserGuide/* New UserGuide component with comprehensive implementation

Assessment against linked issues

Objective Addressed Explanation
New User Guide component
Animated cursor moving Implemented in UserGuide.tsx
Animation tour position Dynamic positioning using @floating-ui/react-dom

Suggested Labels

documentation

Suggested Reviewers

  • vladko-uxds

📜 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 df23009 and 5c99c97.

📒 Files selected for processing (1)
  • packages/react-components/src/components/UserGuide/UserGuide.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/react-components/src/components/UserGuide/UserGuide.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: chromatic-deployment

🪧 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.

@marcinsawicki marcinsawicki self-assigned this Jan 22, 2025
@marcinsawicki marcinsawicki added feature New feature or request design UI/UX oriented issue labels Jan 22, 2025
@marcinsawicki marcinsawicki added this to the Cycle #11 milestone Jan 22, 2025
@marcinsawicki marcinsawicki linked an issue Jan 22, 2025 that may be closed by this pull request
2 tasks
@marcinsawicki marcinsawicki marked this pull request as ready for review January 22, 2025 13:44
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: 3

🧹 Nitpick comments (18)
packages/react-components/src/components/UserGuide/UserGuide.tsx (1)

95-95: Avoid including containerRef.current in dependency array

Including containerRef.current in the dependency array of the useEffect hook can cause unnecessary re-renders. Refs are mutable and should not be dependencies.

Apply this diff to adjust the dependency array:

-useEffect(() => {
-  // ...
-}, [parentElement, containerRef.current, isVisible]);
+useEffect(() => {
+  // ...
+}, [parentElement, isVisible]);
packages/react-components/src/components/UserGuide/components/types.ts (1)

22-29: Consider providing default values for video options.

The video configuration could benefit from having sensible defaults, especially for common settings like playsInline, muted, and autoPlay.

packages/react-components/src/components/AppFrame/components/NavigationItem/NavigationItem.tsx (2)

81-81: Remove commented code.

Clean up the codebase by removing the commented-out id assignment.

-              // id={id}

54-54: Consider keeping id on the anchor element.

The id might be more semantically appropriate on the anchor element since it's the interactive part of the navigation item.

-      id={id}
packages/react-components/src/components/UserGuide/components/UserGuideStep.tsx (2)

53-62: Add accessibility attributes to video element.

The video element should include aria-label for screen readers.

 <video
   src={video.src}
   playsInline={video.playsInline}
   autoPlay={video.autoPlay}
   muted={video.muted}
   loop={video.loop}
   controls={video.controls}
+  aria-label={video.alt || "User guide video"}
 />

66-91: Enhance keyboard navigation in footer.

Consider adding tabIndex and keyboard event handlers for better accessibility.

packages/react-components/src/components/UserGuide/UserGuide.stories.tsx (3)

50-93: Refactor reducer to use switch statement.

The current if-else chain makes the reducer hard to maintain. Consider using a switch statement for better readability and maintainability.

-    if (action.type === 'reference1') {
-      return {
-        ...state,
-        reference: 'reference1',
-        cursorPosition: 'left',
-        cursorTiming: 'moderate2',
-      };
-    }
-    if (action.type === 'reference2') {
+    switch (action.type) {
+      case 'reference1':
+        return {
+          ...state,
+          reference: 'reference1',
+          cursorPosition: 'left',
+          cursorTiming: 'moderate2',
+        };
+      case 'reference2':

144-156: Extract repeated text content into constants.

The header and text content is duplicated across multiple UserGuideStep components. Consider extracting these into constants or a configuration object.

const GUIDE_STEPS = {
  reference1: {
    header: "Title text goes here",
    text: "Some text, maximum 210 characters...",
    image: {
      src: beautifulImage,
      alt: 'image'
    }
  },
  // ... other steps
};

Also applies to: 159-171, 174-186


489-497: Consider using a map for step navigation.

The current implementation uses multiple conditional renders. Consider using a map to render steps based on the current reference.

const STEPS_CONFIG = {
  home: {
    header: "This is navigation item",
    text: "",
    nextStep: "archives"
  },
  // ... other steps
};

{Object.entries(STEPS_CONFIG).map(([key, config]) => 
  state.reference === key ? (
    <UserGuideStep
      key={key}
      header={config.header}
      text={config.text}
      currentStep={/* ... */}
      stepMax={9}
      handleClickPrimary={() => dispatch({ type: config.nextStep })}
      handleCloseAction={() => dispatch({ type: 'isVisible' })}
    />
  ) : null
)}

Also applies to: 499-508, 510-519

packages/react-components/src/components/UserGuide/components/UserGuideStep.module.scss (2)

7-8: Use design system variables for gradient colors.

Replace hardcoded colors with design system variables for better maintainability and theme consistency.

-  background: linear-gradient(45deg, #cddef9, #e7e5fd, #f9f2ff);
+  background: linear-gradient(45deg, var(--gradient-start), var(--gradient-middle), var(--gradient-end));

9-9: Consider making the width responsive.

Fixed width of 340px might cause issues on smaller screens. Consider using max-width or relative units.

-  width: 340px;
+  max-width: 340px;
+  width: 100%;
packages/react-components/src/components/UserGuide/stories-helpers.css (2)

19-19: Use spacing variables for consistency.

Replace magic numbers with design system spacing variables:

-  padding: 8px;
+  padding: var(--spacing-2);

-  padding: 19px 24px;
+  padding: var(--spacing-4) var(--spacing-5);

-  margin-left: 16px;
+  margin-left: var(--spacing-4);

Also applies to: 53-53, 59-59


42-42: Consider extracting magic number to a variable.

The fixed height of 56px appears to be a common value for headers. Consider extracting it to a CSS variable.

+:root {
+  --header-height: 56px;
+}

.column-header {
-  height: 56px;
+  height: var(--header-height);
}
packages/react-components/src/components/UserGuide/UserGuide.module.scss (3)

9-23: Consider adding vendor prefixes for better browser compatibility.

Add autoprefixer to ensure styles work consistently across different browsers.

 &__floating {
    @include transitions.durations();

+   -webkit-transition-property: top, bottom, left, right;
    transition-property: top, bottom, left, right;
+   -webkit-transition-timing-function: ease-in-out;
    transition-timing-function: ease-in-out;
    z-index: $stacking-context-level-tooltip;
  }

25-118: Extract magic numbers into variables.

Define variables for commonly used values to improve maintainability.

+ $arrow-offset: 9px;
+ $arrow-position-offset: 43px;
+ $arrow-end-offset: 45px;

 &--left-start {
-  top: 9px;
+  top: $arrow-offset;
   transform: rotate(50deg);
 }

124-139: Use transitions module for cursor animations.

For consistency, leverage the transitions module instead of hardcoding duration.

 &__cursor {
-  transition: transform ease-in-out 0.2s;
+  @include transitions.durations();
+  transition-property: transform;
+  transition-timing-function: ease-in-out;
packages/react-components/src/components/UserGuide/UserGuide.mdx (2)

21-165: Enhance example code maintainability.

  1. Define proper TypeScript interfaces for state and action
  2. Extract repeated UserGuideStep content into a constant
  3. Consider using a map or switch statement instead of multiple if conditions
+interface GuideState {
+  isVisible: boolean;
+  reference: string;
+  cursorPosition: string;
+  cursorTiming: string;
+  elementStyles?: Record<string, unknown>;
+}

+interface GuideAction {
+  type: 'reference1' | 'reference2' | 'reference3' | 'isVisible';
+}

+const GUIDE_STEP_CONTENT = {
+  header: "Title text goes here",
+  text: "Some text, maximum 210 characters...",
+  image: {
+    src: beautifulImage,
+    alt: 'image',
+  },
+};

173-205: Improve documentation clarity.

  1. Fix grammar: "easly" → "easily"
  2. Add concrete examples of style properties
 elementStyles: {
-  // styles properties here
+  border: '2px solid blue',
+  borderRadius: '4px',
+  boxShadow: '0 0 10px rgba(0, 0, 0, 0.1)',
 }
🧰 Tools
🪛 LanguageTool

[style] ~175-~175: Consider a more expressive alternative.
Context: ...t, by passing own styles properties. To do this, you need to enchant the `UserGuid...

(DO_ACHIEVE)


[uncategorized] ~175-~175: You might be missing the article “a” here.
Context: ... enchant the UserGuide component with styles object using the elementStyles prop: ...

(AI_EN_LECTOR_MISSING_DETERMINER_A)

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 05b8b1d and 37f8c99.

⛔ Files ignored due to path filters (2)
  • packages/react-components/src/components/UserGuide/placeholder.png is excluded by !**/*.png
  • packages/react-components/src/stories/assets/cursor.svg is excluded by !**/*.svg
📒 Files selected for processing (32)
  • .stylelintrc.json (1 hunks)
  • packages/react-components/src/components/ActionBar/ActionBar.tsx (1 hunks)
  • packages/react-components/src/components/AppFrame/components/ExpirationCounter/ExpirationCounter.tsx (1 hunks)
  • packages/react-components/src/components/AppFrame/components/NavigationItem/NavigationItem.tsx (2 hunks)
  • packages/react-components/src/components/AppFrame/components/NavigationTopBar/NavigationTopBar.tsx (3 hunks)
  • packages/react-components/src/components/AppFrame/components/NavigationTopBar/examples.tsx (2 hunks)
  • packages/react-components/src/components/AppFrame/stories-helpers.tsx (2 hunks)
  • packages/react-components/src/components/Tooltip/Tooltip.mdx (0 hunks)
  • packages/react-components/src/components/Tooltip/Tooltip.stories.css (0 hunks)
  • packages/react-components/src/components/Tooltip/Tooltip.stories.tsx (1 hunks)
  • packages/react-components/src/components/Tooltip/components/UserGuide/SpotlightOverlay.tsx (0 hunks)
  • packages/react-components/src/components/Tooltip/components/UserGuide/UserGuide.tsx (0 hunks)
  • packages/react-components/src/components/Tooltip/components/UserGuide/UserGuideStep.tsx (0 hunks)
  • packages/react-components/src/components/Tooltip/components/UserGuide/index.ts (0 hunks)
  • packages/react-components/src/components/Tooltip/components/index.ts (0 hunks)
  • packages/react-components/src/components/Tooltip/index.ts (0 hunks)
  • packages/react-components/src/components/UserGuide/UserGuide.mdx (1 hunks)
  • packages/react-components/src/components/UserGuide/UserGuide.module.scss (1 hunks)
  • packages/react-components/src/components/UserGuide/UserGuide.stories.css (1 hunks)
  • packages/react-components/src/components/UserGuide/UserGuide.stories.tsx (1 hunks)
  • packages/react-components/src/components/UserGuide/UserGuide.tsx (1 hunks)
  • packages/react-components/src/components/UserGuide/components/UserGuideStep.module.scss (1 hunks)
  • packages/react-components/src/components/UserGuide/components/UserGuideStep.tsx (1 hunks)
  • packages/react-components/src/components/UserGuide/components/types.ts (1 hunks)
  • packages/react-components/src/components/UserGuide/index.ts (1 hunks)
  • packages/react-components/src/components/UserGuide/stories-helpers.css (1 hunks)
  • packages/react-components/src/components/UserGuide/stories-helpers.tsx (1 hunks)
  • packages/react-components/src/components/UserGuide/styles/transitions.scss (1 hunks)
  • packages/react-components/src/components/UserGuide/types.ts (1 hunks)
  • packages/react-components/src/components/UserGuide/virtualElementReference.ts (1 hunks)
  • packages/react-components/src/foundations/transition.css (1 hunks)
  • packages/react-components/src/utils/types.ts (1 hunks)
💤 Files with no reviewable changes (8)
  • packages/react-components/src/components/Tooltip/components/index.ts
  • packages/react-components/src/components/Tooltip/Tooltip.stories.css
  • packages/react-components/src/components/Tooltip/index.ts
  • packages/react-components/src/components/Tooltip/components/UserGuide/index.ts
  • packages/react-components/src/components/Tooltip/components/UserGuide/UserGuideStep.tsx
  • packages/react-components/src/components/Tooltip/components/UserGuide/SpotlightOverlay.tsx
  • packages/react-components/src/components/Tooltip/components/UserGuide/UserGuide.tsx
  • packages/react-components/src/components/Tooltip/Tooltip.mdx
✅ Files skipped from review due to trivial changes (2)
  • packages/react-components/src/components/UserGuide/styles/transitions.scss
  • packages/react-components/src/components/UserGuide/index.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/react-components/src/components/UserGuide/stories-helpers.tsx

[error] 21-21: Missing key property for this element in iterable.

The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.

(lint/correctness/useJsxKeyInIterable)

🪛 LanguageTool
packages/react-components/src/components/UserGuide/UserGuide.mdx

[uncategorized] ~171-~171: Possible missing preposition found.
Context: ...ed next to the element the guide points to. We suggest using UserGuideStep. ## ...

(AI_EN_LECTOR_MISSING_PREPOSITION)


[style] ~175-~175: Consider a more expressive alternative.
Context: ...t, by passing own styles properties. To do this, you need to enchant the `UserGuid...

(DO_ACHIEVE)


[uncategorized] ~175-~175: You might be missing the article “a” here.
Context: ... enchant the UserGuide component with styles object using the elementStyles prop: ...

(AI_EN_LECTOR_MISSING_DETERMINER_A)

🔇 Additional comments (17)
packages/react-components/src/components/UserGuide/UserGuide.tsx (1)

19-204: Well-structured UserGuide component

The UserGuide component is thoughtfully implemented with effective use of React hooks and floating UI positioning. The logic ensures dynamic positioning and cloning of the parent element for contextual guidance.

packages/react-components/src/utils/types.ts (1)

6-9: Appropriate addition of id prop

Adding the optional id prop enhances component identification without impacting existing functionality.

packages/react-components/src/components/UserGuide/types.ts (1)

1-18: Clear and precise type definitions

The type definitions for IUserGuide and CursorTiming are well-structured, providing clarity for the component's props.

packages/react-components/src/components/UserGuide/virtualElementReference.ts (1)

11-14: Using Math.ceil prevents fractional pixel values.

The change from Math.round to Math.ceil ensures consistent rendering by preventing sub-pixel calculations, which can cause visual artifacts.

packages/react-components/src/components/UserGuide/components/types.ts (1)

3-46: Well-structured interface with comprehensive documentation.

The interface is logically organized and thoroughly documented. Good job including keyboard event handling for accessibility.

packages/react-components/src/components/AppFrame/components/ExpirationCounter/ExpirationCounter.tsx (1)

41-41: Good accessibility enhancement.

Adding the id attribute improves both testability and accessibility of the component.

packages/react-components/src/components/AppFrame/components/NavigationTopBar/examples.tsx (1)

9-10: LGTM!

The addition of the optional id prop enhances component identification and accessibility.

Also applies to: 35-35

packages/react-components/src/components/UserGuide/stories-helpers.tsx (1)

64-68: Implement proper callback for ActionBar options.

Replace the empty callback with a proper implementation or add error handling.

-            options={getDefaultOptions(() => {})}
+            options={getDefaultOptions(() => {
+              console.warn('Action not implemented');
+            })}
packages/react-components/src/components/Tooltip/Tooltip.stories.tsx (1)

11-11: LGTM!

Clean removal of UserGuide from Tooltip context, aligning with the PR objective of making UserGuide a standalone component.

packages/react-components/src/components/ActionBar/ActionBar.tsx (1)

138-138: LGTM! Good accessibility enhancement.

The unique ID for the menu button improves component accessibility and testability.

packages/react-components/src/components/AppFrame/components/NavigationTopBar/NavigationTopBar.tsx (2)

38-38: LGTM! Consistent accessibility improvements.

The id prop additions align with the component-wide accessibility enhancements.

Also applies to: 43-43


99-99: LGTM! Good accessibility implementation.

The id prop is properly implemented in both the NavigationTopBar and NavigationTopBarAlert components.

Also applies to: 183-183

packages/react-components/src/components/AppFrame/stories-helpers.tsx (1)

167-168: LGTM! Good prop propagation.

The id prop is properly typed as optional and correctly passed down to child components.

Also applies to: 201-201

.stylelintrc.json (1)

12-13: Verify the necessity of disabling position checks for @import rules.

Disabling no-invalid-position-at-import-rule could lead to style loading issues. Consider keeping the rule enabled unless there's a specific use case requiring its removal.

packages/react-components/src/components/UserGuide/UserGuide.stories.css (1)

1-28: LGTM! Well-structured CSS with proper use of design system variables.

Clean implementation using flexbox for layout and consistent spacing.

packages/react-components/src/components/UserGuide/UserGuide.module.scss (1)

1-8: Well-structured setup with modular imports!

Good use of transitions module and stacking context utilities for consistent styling.

packages/react-components/src/components/UserGuide/UserGuide.mdx (1)

1-20: Well-structured documentation with clear navigation!

Comment on lines +22 to +30
useEffect(() => {
if (handleCloseAction) {
document.addEventListener('keydown', handleCloseAction);

return () => {
document.removeEventListener('keydown', handleCloseAction);
};
}
}, []);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add handleCloseAction to useEffect dependencies.

The empty dependency array could cause stale closure issues if handleCloseAction changes during component lifecycle.

-  }, []);
+  }, [handleCloseAction]);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
useEffect(() => {
if (handleCloseAction) {
document.addEventListener('keydown', handleCloseAction);
return () => {
document.removeEventListener('keydown', handleCloseAction);
};
}
}, []);
useEffect(() => {
if (handleCloseAction) {
document.addEventListener('keydown', handleCloseAction);
return () => {
document.removeEventListener('keydown', handleCloseAction);
};
}
}, [handleCloseAction]);

Comment on lines +20 to +31
{[...Array(7)].map((_, index) => (
<div id={`chats-list-item-${index}`} className="chats-list-item">
<SkeletonWrapper>
<SkeletonAvatar size={36} />
<SkeletonWrapper vertical>
<SkeletonText width={100} />
<SkeletonText />
</SkeletonWrapper>
</SkeletonWrapper>
</div>
))}
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add key prop to mapped elements.

React requires a unique key prop for elements in an array to optimize rendering and prevent potential issues.

-            <div id={`chats-list-item-${index}`} className="chats-list-item">
+            <div key={`chats-list-item-${index}`} id={`chats-list-item-${index}`} className="chats-list-item">
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{[...Array(7)].map((_, index) => (
<div id={`chats-list-item-${index}`} className="chats-list-item">
<SkeletonWrapper>
<SkeletonAvatar size={36} />
<SkeletonWrapper vertical>
<SkeletonText width={100} />
<SkeletonText />
</SkeletonWrapper>
</SkeletonWrapper>
</div>
))}
</div>
{[...Array(7)].map((_, index) => (
<div key={`chats-list-item-${index}`} id={`chats-list-item-${index}`} className="chats-list-item">
<SkeletonWrapper>
<SkeletonAvatar size={36} />
<SkeletonWrapper vertical>
<SkeletonText width={100} />
<SkeletonText />
</SkeletonWrapper>
</SkeletonWrapper>
</div>
))}
</div>
🧰 Tools
🪛 Biome (1.9.4)

[error] 21-21: Missing key property for this element in iterable.

The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.

(lint/correctness/useJsxKeyInIterable)

@@ -3,6 +3,7 @@
--transition-duration-fast-1: 100ms;
--transition-duration-fast-2: 200ms;
--transition-duration-moderate-1: 300ms;
--transition-duration-moderate-2: 500ms;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add reduced motion handling for new duration.

The new --transition-duration-moderate-2 needs to be reset in both @media (prefers-reduced-motion) and .no-animations blocks.

 @media (prefers-reduced-motion) {
   :root {
     --transition-duration-fast-1: 0ms;
     --transition-duration-fast-2: 0ms;
     --transition-duration-moderate-1: 0ms;
+    --transition-duration-moderate-2: 0ms;
     --delay-instant: 0ms;
     --delay-moderate: 0ms;
     --delay-slow: 0ms;
   }
 }

 .no-animations {
   --transition-duration-fast-1: 0ms;
   --transition-duration-fast-2: 0ms;
   --transition-duration-moderate-1: 0ms;
+  --transition-duration-moderate-2: 0ms;
   --delay-instant: 0ms;
   --delay-moderate: 0ms;
   --delay-slow: 0ms;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
--transition-duration-moderate-2: 500ms;
--transition-duration-moderate-2: 500ms;
@media (prefers-reduced-motion) {
:root {
--transition-duration-fast-1: 0ms;
--transition-duration-fast-2: 0ms;
--transition-duration-moderate-1: 0ms;
--transition-duration-moderate-2: 0ms;
--delay-instant: 0ms;
--delay-moderate: 0ms;
--delay-slow: 0ms;
}
}
.no-animations {
--transition-duration-fast-1: 0ms;
--transition-duration-fast-2: 0ms;
--transition-duration-moderate-1: 0ms;
--transition-duration-moderate-2: 0ms;
--delay-instant: 0ms;
--delay-moderate: 0ms;
--delay-slow: 0ms;
}

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

🧹 Nitpick comments (2)
packages/react-components/src/components/UserGuide/types.ts (2)

5-5: Consider using constants for timing values.

Define timing values as constants to maintain consistency and enable reuse across the codebase.

+const CURSOR_TIMINGS = {
+  FAST1: 'fast1',
+  FAST2: 'fast2',
+  MODERATE1: 'moderate1',
+  MODERATE2: 'moderate2',
+} as const;
-export type CursorTiming = 'fast1' | 'fast2' | 'moderate1' | 'moderate2';
+export type CursorTiming = typeof CURSOR_TIMINGS[keyof typeof CURSOR_TIMINGS];

25-27: Consider renaming parentElementName to targetElementId.

The current name suggests a parent-child relationship, but it's actually targeting an element to highlight.

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 37f8c99 and df23009.

📒 Files selected for processing (2)
  • packages/react-components/src/components/UserGuide/UserGuide.stories.tsx (1 hunks)
  • packages/react-components/src/components/UserGuide/types.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/react-components/src/components/UserGuide/UserGuide.stories.tsx
🔇 Additional comments (2)
packages/react-components/src/components/UserGuide/types.ts (2)

1-3: LGTM! Clean and necessary imports.


7-32: Well-structured interface with clear documentation!

The interface provides a flexible API with all optional properties, making it easy to use and extend.

Copy link

@vladko-uxds vladko-uxds left a comment

Choose a reason for hiding this comment

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

👍

@@ -77,6 +78,7 @@ export const NavigationItem: React.FC<INavigationItemProps> = ({
triggerRenderer={
<>
<a
// id={id}
tabIndex={disabled ? -1 : 0}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please remove unnecessary comment

@@ -35,11 +35,12 @@ export const NavigationTopBar = ({
children,
className,
additionalNodes,
id,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Id shouldn't be optional here like in the other components?

}

&--right-end {
top: calc(100% - 45px - 9px);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

why not 100% - 54px ?

}

&--left-end {
top: calc(100% - 45px - 9px);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here

styles[`${baseClass}__guide__arrow--${cursorTiming}`]
)}
>
<svg
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we extract this svg to a separate component?

.chats-list-item {
display: flex;
flex-direction: row;
padding: 19px 24px;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

24px => --spacing-6

.chats-list-item-content {
display: flex;
flex-direction: column;
margin-left: 16px;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

16px => --spacing-4

Copy link
Contributor Author

@JoannaSikora JoannaSikora left a comment

Choose a reason for hiding this comment

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

👍🏼

Comment on lines +127 to +130
<div
className={cx(styles[`${baseClass}__overlay`])}
style={{
display: isVisible ? 'block' : 'none',
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add an option for a custom z-index on the overlay? Sometimes some elements have bigger zindex than overlay itself. I assume that the same index will be used for cloned element

export const UserGuide: FC<PropsWithChildren<IUserGuide>> = ({
className,
children,
cursorPosition = 'bottom',
Copy link
Contributor

@MateuszRomek MateuszRomek Jan 23, 2025

Choose a reason for hiding this comment

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

In this case, do we completely abandon the previous look and behavior of UserGuide? I ask about this because Agent App uses the current UserGuide component in its current app tours (which will stay), so this is partly a breaking change and a possible regression for all tours in the application, due to new element cloning and UI of this guide.

Do you foresee any possible problems with previous use of user guide?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design UI/UX oriented issue feature New feature or request
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

[User Guide ] - new look and feel (new kind) [User Guide] - highlight box update
4 participants