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: Add tooltips to various UI elements (fixes #148). #160

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

Conversation

junhaoliao
Copy link
Member

@junhaoliao junhaoliao commented Dec 26, 2024

Description

  1. Add variant={"outlined"} and arrow={true} to global tooltip props and remove any such specializations from existing components.
  2. Refactor MenuBar buttons into reusable MenuBarIconButton.
  3. Add tooltips to:
    • page navigation buttons
    • export logs button
    • click to copy link button
    • buttons in the search panel, which include:
      • Collapse / Expand all
      • Case sensitivity toggle
      • Wildcard toggle
  4. Adjust the placements of the menu bar button tooltips. Note the "Open file" button's tooltip is now no longer covering the file name when expanded.
  5. Wrap buttons with tooltips in span elements so that the tooltips can still be expanded even when the buttons are disabled.

Validation performed

  1. Started the debug server as instructed at https://docs.yscope.com/yscope-log-viewer/main/dev-guide/building-getting-started#running-in-development
  2. Loaded demo http://localhost:3010?filePath=https://yscope.s3.us-east-2.amazonaws.com/sample-logs/yarn-ubuntu-resourcemanager-ip-172-31-17-135.log.1.clp.zst
  3. Hovered onto below buttons and ensure the visual effects (i.e., outlined with arrow) were as expected:
    • open file button
    • page navigation buttons
    • export logs button
    • click to copy link button
    • sidebar tab buttons
    • the options / checkboxes in the log level selector
    • buttons in the search panel, which include:
      • Collapse / Expand all
      • Case sensitivity toggle
      • Wildcard toggle
  4. Loaded the app without a file (i.e., http://localhost:3010). Hovered onto the above buttons and ensured the tooltips were shown even for the ones that were disabled.

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Introduced a new MenuBarIconButton component with tooltip support.
    • Enhanced StatusBar with a tooltip for the log event button.
    • Added tooltips to various buttons in the SearchTabPanel for improved user interaction.
  • Bug Fixes

    • Removed unnecessary properties from the Tooltip in the TabButton component.
  • Chores

    • Replaced SmallIconButton with MenuBarIconButton in various components.
    • Updated MenuBar to streamline button functionality.
  • Documentation

    • Added default properties for JoyTooltip in the theme configuration.

Copy link

coderabbitai bot commented Dec 26, 2024

Walkthrough

This pull request introduces a series of modifications to UI components across multiple files in the project. Key changes include the creation of the new MenuBarIconButton component, which replaces the existing SmallIconButton and enhances tooltip functionality. The updates standardize button interactions in components such as the NavigationBar, MenuBar, and StatusBar. Additionally, the theme configuration for tooltips has been adjusted to provide consistent default styling.

Changes

File Change Summary
src/components/CentralContainer/Sidebar/SidebarTabs/TabButton.tsx Removed arrow and variant properties from Tooltip component
src/components/MenuBar/ExportLogsButton.tsx Replaced SmallIconButton with MenuBarIconButton and added title prop
src/components/MenuBar/MenuBarIconButton.tsx New component created with tooltip functionality and extended IconButton
src/components/MenuBar/NavigationBar.tsx Replaced IconButton with MenuBarIconButton for navigation buttons
src/components/MenuBar/SmallIconButton.tsx Entire file removed
src/components/MenuBar/index.tsx Replaced Tooltip and IconButton with MenuBarIconButton
src/components/StatusBar/index.tsx Added Tooltip to button displaying log event number
src/components/theme.tsx Added default configuration for JoyTooltip with variant and arrow properties
src/components/CentralContainer/Sidebar/SidebarTabs/PanelTitleButton.tsx Added Tooltip wrapper around IconButton with title prop
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx Introduced Tooltip for buttons and refactored collapse/expand functionality

Sequence Diagram

sequenceDiagram
    participant UI as User Interface
    participant MB as MenuBarIconButton
    participant TT as Tooltip
    
    UI->>MB: Hover/Interact
    MB->>TT: Trigger Tooltip
    TT-->>UI: Display Tooltip
Loading

Possibly Related PRs

Suggested Reviewers

  • davemarco
  • Henry8192

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.

@junhaoliao junhaoliao changed the title Add default "outlined" variant to JoyTooltip component. feat: Add tooltips to various UI elements. Dec 26, 2024
@junhaoliao junhaoliao changed the title feat: Add tooltips to various UI elements. feat: Add tooltips to various UI elements (fixes #148). Dec 26, 2024
…still be expanded even when the buttons are disabled.
@junhaoliao junhaoliao requested a review from Henry8192 December 26, 2024 06:45
@junhaoliao junhaoliao marked this pull request as ready for review December 26, 2024 06:45
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)
src/components/MenuBar/index.tsx (2)

50-54: Consider accessibility improvements for tooltips.
The usage of MenuBarIconButton along with title and tooltipPlacement={"bottom-start"} is effective for discoverability. However, ensure that screen reader users can also access these tooltips by providing additional ARIA attributes (e.g., aria-label or aria-describedby).


56-56: Confirm icon accessibility.
Using <FolderOpenIcon /> is correct for visual representation. For fully accessible design, consider providing an accessible name (e.g., via aria-label) in case tooltips are not read by the screen reader.

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between af322a6 and c2af7c6.

📒 Files selected for processing (8)
  • src/components/CentralContainer/Sidebar/SidebarTabs/TabButton.tsx (0 hunks)
  • src/components/MenuBar/ExportLogsButton.tsx (3 hunks)
  • src/components/MenuBar/MenuBarIconButton.tsx (1 hunks)
  • src/components/MenuBar/NavigationBar.tsx (3 hunks)
  • src/components/MenuBar/SmallIconButton.tsx (0 hunks)
  • src/components/MenuBar/index.tsx (2 hunks)
  • src/components/StatusBar/index.tsx (2 hunks)
  • src/components/theme.tsx (1 hunks)
💤 Files with no reviewable changes (2)
  • src/components/MenuBar/SmallIconButton.tsx
  • src/components/CentralContainer/Sidebar/SidebarTabs/TabButton.tsx
🧰 Additional context used
📓 Path-based instructions (6)
src/components/MenuBar/index.tsx (1)

Pattern **/*.{cpp,hpp,java,js,jsx,ts,tsx}: - Prefer false == <expression> rather than !<expression>.

src/components/theme.tsx (1)

Pattern **/*.{cpp,hpp,java,js,jsx,ts,tsx}: - Prefer false == <expression> rather than !<expression>.

src/components/StatusBar/index.tsx (1)

Pattern **/*.{cpp,hpp,java,js,jsx,ts,tsx}: - Prefer false == <expression> rather than !<expression>.

src/components/MenuBar/NavigationBar.tsx (1)

Pattern **/*.{cpp,hpp,java,js,jsx,ts,tsx}: - Prefer false == <expression> rather than !<expression>.

src/components/MenuBar/MenuBarIconButton.tsx (1)

Pattern **/*.{cpp,hpp,java,js,jsx,ts,tsx}: - Prefer false == <expression> rather than !<expression>.

src/components/MenuBar/ExportLogsButton.tsx (1)

Pattern **/*.{cpp,hpp,java,js,jsx,ts,tsx}: - Prefer false == <expression> rather than !<expression>.

🔇 Additional comments (17)
src/components/theme.tsx (1)

58-63: Looks good.

These default props for JoyTooltip appear consistent with the existing theme customizations. The outlined variant and arrow style are correctly integrated, and no conflicts or code smells are observed.

src/components/MenuBar/MenuBarIconButton.tsx (5)

1-6: Imports look well organised.

No issues identified. They comply with recommended import ordering and usage.


9-11: Interface extension is clear.

Defining MenuBarIconButtonProps to extend IconButtonProps is an effective way to ensure consistent prop usage in your custom component.


13-21: Documentation is succinct.

The JSDoc block accurately describes props and makes it easy for maintainers to understand the component interface.


22-37: Tooltip usage is straightforward and consistent.

Wrapping the button in a <span> ensures the tooltip is active when the button is disabled—following best practices for accessibility.


39-39: Export default is consistent with the kit's naming.

This suits the component-based architecture and fosters reusability.

src/components/StatusBar/index.tsx (2)

6-6: Tooltip import aligns with the global UI enhancements.

No conflicts found, and it maintains consistency with the rest of the codebase.


44-59: Tooltip wrapping ensures visibility even when disabled.

This approach provides a better user experience, particularly for communicating button functionality or restrictions.

src/components/MenuBar/ExportLogsButton.tsx (2)

20-20: The new import is an improvement.

Switching to MenuBarIconButton unifies button usage in the menu bar, promoting a more consistent UX.


Line range hint 32-60: Title prop enhances usability.

Providing a tooltip label (“Export logs”) clarifies button functionality, especially for first-time users.

src/components/MenuBar/NavigationBar.tsx (6)

3-3: Removal of size from ButtonGroup reduces potential conflicts.

Keeping default or context-based sizing within child components is often more flexible.


17-17: Importing MenuBarIconButton centralises logic.

It's good practice to use a unified component for consistent styling and tooltip handling.


50-56: Consistent naming for the data attribute and props.

Clear naming ensures that FIRST_PAGE action is easily identifiable when handling events.


57-63: Tooltip for previous page button is descriptive.

A consistent tooltip name helps guide users through the navigation.


67-73: Next page button usage is straightforward.

Reusing the same component fosters uniformity across the navigation bar.


74-80: Last page button is aligned with the pattern of preceding buttons.

All navigation buttons follow the same design language, creating a streamlined experience.

src/components/MenuBar/index.tsx (1)

19-19: Imports look good.
Importing MenuBarIconButton is a clear step towards consolidating button and tooltip logic in one place. This improves maintainability and consistency.

@junhaoliao junhaoliao marked this pull request as draft December 26, 2024 06:47
@junhaoliao junhaoliao marked this pull request as ready for review December 26, 2024 07:11
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)
src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx (1)

71-74: Consider preferring “false == v”
According to the coding guidelines, favour using false == v instead of !v. This helps maintain consistency across the codebase.

- setIsAllExpanded((v) => !v);
+ setIsAllExpanded((v) => false == v);
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between c2af7c6 and 5727904.

📒 Files selected for processing (3)
  • src/components/CentralContainer/Sidebar/SidebarTabs/PanelTitleButton.tsx (2 hunks)
  • src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx (4 hunks)
  • src/components/theme.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/theme.tsx
🧰 Additional context used
📓 Path-based instructions (2)
src/components/CentralContainer/Sidebar/SidebarTabs/PanelTitleButton.tsx (1)

Pattern **/*.{cpp,hpp,java,js,jsx,ts,tsx}: - Prefer false == <expression> rather than !<expression>.

src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx (1)

Pattern **/*.{cpp,hpp,java,js,jsx,ts,tsx}: - Prefer false == <expression> rather than !<expression>.

🔇 Additional comments (9)
src/components/CentralContainer/Sidebar/SidebarTabs/PanelTitleButton.tsx (3)

4-4: Nice import usage for Tooltip
The usage of the Tooltip import aligns with the new tooltip feature introduced in the PR.


14-16: Good clarity in doc comments
Explicitly documenting className, title, and rest improves maintainability and eases onboarding for new contributors.


19-31: Comprehensive approach to Tooltip wrapping
By wrapping the IconButton in both a Tooltip and a span, you allow tooltips to display for disabled states. This is a recommended UI pattern, ensuring consistent and clear feedback to end-users.

src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/index.tsx (6)

13-13: Tooltip import looks correct
Importing Tooltip from @mui/joy is consistent with other UI changes for tooltips, ensuring a standard visual style.


100-101: Clear naming for disabled state
Using isQueryInputBoxDisabled is descriptive and straightforward, enhancing code clarity around UI state handling.


107-112: Tooltip titles enhance clarity
Providing distinct title text for “Collapse all” or “Expand all” improves user understanding of the button’s function. Great job!


128-128: Appropriate disable logic
Disabling the ToggleButtonGroup conditionally with disabled={isQueryInputBoxDisabled} is clean, ensuring users can’t interact with query options when unavailable.


135-155: Span wrapper enables tooltips on disabled buttons
Wrapping disabled IconButton elements with a span preserves tooltip functionality, which is a recommended pattern in MUI.


159-162: Slot props usage is straightforward
Providing textarea and endDecorator overrides in slotProps simplifies component customisation while keeping the code readable. Nicely implemented.

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.

1 participant