-
Notifications
You must be signed in to change notification settings - Fork 61
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(app-sys): add tab title option #17263
Conversation
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
warning [email protected]: This version is no longer supported. Please see https://eslint.org/version-support for other options. WalkthroughThe pull request introduces modifications to two files in the application's type and UI shell libraries. The changes primarily focus on enhancing localization support by adding an optional Changes
Sequence DiagramsequenceDiagram
participant UI as User Interface
participant Hook as useApplicationTitle
participant Section as Active Section
UI->>Section: Retrieve section details
Section-->>Hook: Provide section data
Hook->Hook: Check for tabTitle
alt tabTitle exists
Hook->>Hook: Use tabTitle
else tabTitle is undefined
Hook->>Hook: Fallback to title
end
Hook->>UI: Set document title
The sequence diagram illustrates how the Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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
Documentation and Community
|
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: 0
🧹 Nitpick comments (1)
libs/application/ui-shell/src/hooks/useApplicationTitle.ts (1)
35-40
: Consider destructuring in a single line for better readability.The implementation is correct, but the variable declarations could be more concise.
Consider this refactoring:
- const tabTitle = activeSection.tabTitle - const title = activeSection.title + const { tabTitle, title } = activeSection
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libs/application/types/src/lib/Form.ts
(1 hunks)libs/application/ui-shell/src/hooks/useApplicationTitle.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
libs/application/types/src/lib/Form.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/ui-shell/src/hooks/useApplicationTitle.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (2)
libs/application/types/src/lib/Form.ts (1)
113-113
: LGTM! Type definitions are well-structured and maintain backward compatibility.
The addition of the optional tabTitle
property to both interfaces is type-safe and follows TypeScript best practices. The use of FormTextWithLocale
ensures proper localization support.
Also applies to: 119-119
libs/application/ui-shell/src/hooks/useApplicationTitle.ts (1)
35-40
: Verify the impact on existing section titles.
The implementation looks good, but let's verify that existing sections without tabTitle continue to display correctly.
✅ Verification successful
Existing sections continue to display correctly with title property
Based on the extensive codebase analysis, all existing section implementations consistently use the title
property, and the new code correctly falls back to title
when tabTitle
is not present (tabTitle || title
). The search results show numerous section definitions across the codebase that rely on the title
property, and the implementation maintains backward compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for sections using only title property
# This helps verify backward compatibility
# Search for section definitions to ensure existing sections work
ast-grep --pattern 'type: FormItemTypes.SECTION, title: $_'
Length of output: 61
Script:
#!/bin/bash
# Let's try a different approach to find section definitions
# Search for section type declarations and their title/tabTitle usage
# Search for section definitions with rg for better context
rg "type.*FormItemTypes\.SECTION" -A 5
# Also search for interfaces/types that define section properties
ast-grep --pattern 'interface $_ {
$$$
title?: $_
tabTitle?: $_
$$$
}'
Length of output: 2481
Script:
#!/bin/bash
# Let's examine the actual section definitions in the codebase
# and look for title/tabTitle usage patterns
# Search for section definitions with their properties
rg "buildSection\({" -A 10
# Also look for any direct section object definitions
rg "type:\s+FormItemTypes\.SECTION,\s*\n.*title:" -A 5
Length of output: 371
Script:
#!/bin/bash
# Let's try a different approach with fixed regex patterns
# Search for section definitions and their properties
rg "buildSection\(" -A 10
# Look for section type declarations with multiline support
rg -U "type:\s+FormItemTypes\.SECTION.*title:" -A 5
# Also check for any test files that might contain section definitions
rg "test|spec" -t ts -l | xargs rg "FormItemTypes\.SECTION" -A 5
Length of output: 73103
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17263 +/- ##
=======================================
Coverage 35.73% 35.74%
=======================================
Files 6941 6941
Lines 148395 148398 +3
Branches 42331 42332 +1
=======================================
+ Hits 53035 53038 +3
Misses 95360 95360 Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 21 Total Test Services: 0 Failed, 20 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (1)
|
What
Option to set the title in the tab for sections and subsections
Why
It is possible to have an empty stepper and since the title for the tab is derived from there, this option is needed
Checklist:
Summary by CodeRabbit
tabTitle
, to theSection
andSubSection
interfaces.tabTitle
over the existing title property.