-
Notifications
You must be signed in to change notification settings - Fork 15
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
fix:replacing page title from Dashboard to Home #3270
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request updates the title property of several constants and methods related to the side menu in the application. Specifically, it changes the title from "Dashboard" to "Home" in various data structures and the corresponding component methods. Additionally, the test suite for the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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: 0
🧹 Outside diff range comments (4)
src/app/core/mock-data/sidemenu-item.data.ts (1)
Line range hint
4-13
: Your code structure is mass, but let me give you a small suggestion!Hey nanba, your code is looking sharp like my signature sunglasses! But to make it even more powerful, consider adding a type for the route array.
export const sidemenuItemData1: SidemenuItem = deepFreeze({ title: 'Home', isVisible: true, icon: 'dashboard', - route: ['/', 'enterprise', 'my_dashboard'], + route: ['/', 'enterprise', 'my_dashboard'] as const, disabled: false, isDropdownOpen: false, dropdownOptions: [], openLiveChat: false, });This way, TypeScript will treat the route as a tuple with exact values. Simply superb protection against typos!
src/app/shared/components/sidemenu/sidemenu.component.ts (1)
Line range hint
220-224
: Style-ah? Icon needs to match the title, thalaiva!The title has been changed from "Dashboard" to "Home", but the icon still says 'dashboard'. Let's keep it consistent!
- icon: 'dashboard', + icon: 'home',This change should be applied to both occurrences (lines 222 and 301).
Also applies to: 299-303
src/app/shared/components/sidemenu/sidemenu.component.spec.ts (2)
Line range hint
315-386
: Superstar style test coverage, but let's make it even more powerful!The test cases are comprehensive, but we could enhance them further by adding specific assertions for the title change.
Add this additional test case to explicitly verify the title change:
+ it('should show Home instead of Dashboard in primary menu options', () => { + const result = component.getPrimarySidemenuOptions(true); + expect(result[0].title).toBe('Home'); + expect(result[0].icon).toBe('dashboard'); + expect(result[0].route).toEqual(['/', 'enterprise', 'my_dashboard']); + });
Mind it! The change from 'Dashboard' to 'Home' needs attention, style-ah!
While the sidemenu component correctly shows 'Home' with the 'dashboard' icon, there are multiple test files still using 'Dashboard' in their tracking events and descriptions. Here are the places that need updates:
src/app/fyle/dashboard/dashboard.page.spec.ts
: Multiple tracking events using 'Dashboard' as page namesrc/app/fyle/my-advances/my-advances.page.spec.ts
: References to 'Dashboard' in tracking eventssrc/app/fyle/my-expenses/my-expenses.page.spec.ts
: References to 'Dashboard' in test descriptionsThe route path
my_dashboard
can remain unchanged as it's just a URL path, but all user-facing text and tracking events should consistently use "Home" instead of "Dashboard".🔗 Analysis chain
Line range hint
1-600
: Thalaiva says: Let's verify the impact of this change!We should ensure that all references to "Dashboard" have been updated to "Home" across the test suite.
Let me check the actual content to see if we have any inconsistencies between route names and display text, macha! 🕶️
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining instances of "Dashboard" in the test files echo "Checking for remaining 'Dashboard' references in test files..." rg -l "Dashboard" --type-add 'test:*.{spec,test}.*' -t test # Search for any mismatched "Home" and "dashboard" combinations echo "Checking for potential inconsistencies between 'Home' title and 'dashboard' references..." rg -l "title: ['|\"]Home['|\"].*dashboard" --type-add 'test:*.{spec,test}.*' -t testLength of output: 951
Script:
#!/bin/bash # Let's check the actual content of files that have "Dashboard" references echo "Checking content of dashboard.page.spec.ts..." rg -A 5 -B 5 "Dashboard" "src/app/fyle/dashboard/dashboard.page.spec.ts" echo "\nChecking content of sidemenu.component.spec.ts for route references..." rg "dashboard" "src/app/shared/components/sidemenu/sidemenu.component.spec.ts" echo "\nChecking for my_dashboard route patterns across test files..." rg "my_dashboard" --type-add 'test:*.{spec,test}.*' -t testLength of output: 16201
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (4)
src/app/core/mock-data/sidemenu-item.data.ts
(1 hunks)src/app/core/mock-data/sidemenu.data.ts
(4 hunks)src/app/shared/components/sidemenu/sidemenu.component.spec.ts
(1 hunks)src/app/shared/components/sidemenu/sidemenu.component.ts
(3 hunks)
🔇 Additional comments (7)
src/app/core/mock-data/sidemenu-item.data.ts (1)
6-6
: Mind-blowing change, but let's verify the routes, macha!
The title change from 'Dashboard' to 'Home' is super duper perfect! But listen here, thalaiva - we still have the route as my_dashboard
. Style-ah check pannum!
✅ Verification successful
Keep the route as my_dashboard
, macha! It's all part of the grand design!
The route path my_dashboard
is consistently used across the entire codebase for navigation to the home/dashboard page. Changing it would be like trying to stop me from wearing my signature sunglasses, impossible! Here's why:
- The route is deeply integrated in many components:
- Used in app routing module for default redirection
- Referenced in navigation logic across multiple pages (my-reports, team-reports, etc.)
- Part of deep linking and authentication flows
- Used in sidemenu configuration
The title change from 'Dashboard' to 'Home' is purely cosmetic and doesn't require route changes. Just like how I change my style but keep my swag! 😎
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if route paths need to be updated to match the new title
# Search for dashboard-related routes and components
rg -i "dashboard" --type ts
# Search for route configurations
ast-grep --pattern 'route = [$$$]'
Length of output: 51843
src/app/core/mock-data/sidemenu.data.ts (2)
Line range hint 1-230
: Superstar style code organization, but watch out for the freeze!
Mind it! You're using deepFreeze
like I use my sunglasses - with absolute style! But remember, once frozen, these constants are as immutable as my dialogue delivery. Make sure all consuming components are ready for this Home makeover!
Let the power of verification flow through our veins:
✅ Verification successful
No worries, partner! These frozen constants are as safe as a perfectly choreographed action sequence!
The verification shows these constants are only used in sidemenu.component.spec.ts
for testing purposes. Like a well-rehearsed stunt, the deepFreeze
is doing exactly what it should - keeping our test data immutable and predictable.
Let me do one more stylish verification to make sure we're not missing anything:
Mind it! These frozen constants are as cool as my signature sunglasses flip - they're only for testing!
Like a perfectly executed stunt sequence, our verification reveals that these deepFreeze
constants are used exclusively in sidemenu.component.spec.ts
for test scenarios. The actual sidemenu.component.ts
generates its menu items dynamically based on user permissions and settings - a true superstar performance!
The freezing won't affect the production code because:
- The constants are only imported in the spec file
- The real component builds its menu through service calls and user settings
- No other components are depending on these frozen constants
Just like how I say "When I do something, I do it style!", these tests are doing their job with style - keeping our test data immutable and predictable!
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find components using these constants
# Search for imports of sidemenu.data
rg "from.*sidemenu\.data" --type ts
# Search for direct usage of the constants
rg "sidemenuData1|PrimaryOptionsRes1|getPrimarySidemenuOptionsRes1|setSideMenuRes" --type ts
Length of output: 2211
Script:
#!/bin/bash
# Let's check if the actual sidemenu component uses any of these constants
rg -t ts "sidemenuData1|PrimaryOptionsRes1|getPrimarySidemenuOptionsRes1|setSideMenuRes" src/app/shared/components/sidemenu/sidemenu.component.ts
# And let's see what the actual component looks like
cat src/app/shared/components/sidemenu/sidemenu.component.ts
Length of output: 12683
7-7
: 🧹 Nitpick (assertive)
Mind-blowing transformation, but let's make it perfect!
Hey there! Like how I punch through walls with style, you've punched through the codebase changing "Dashboard" to "Home". But listen here, partner - the route paths still say 'my_dashboard'. That's like wearing a Superman costume but calling yourself Batman!
Let me help you verify this across the galaxy... I mean, codebase:
Consider updating the route paths to match the new naming:
- route: ['/', 'enterprise', 'my_dashboard'],
+ route: ['/', 'enterprise', 'home'],
Also applies to: 28-28, 94-94, 161-161
src/app/shared/components/sidemenu/sidemenu.component.ts (3)
Line range hint 220-224
: Superstar style approval! Mind it!
The changes perfectly align with the PR objective to replace "Dashboard" with "Home". The implementation is clean and maintains all existing functionality. The suggested improvements about icon and route consistency can be addressed in a follow-up PR.
Also applies to: 299-303
Line range hint 220-224
: Kabali says: Route path doesn't match the new title, boss!
While we changed the title to "Home", the route path still contains 'my_dashboard'. For better maintainability and consistency:
- Consider updating the route path
- Ensure all related route references are updated
Also applies to: 299-303
4-4
: Mind-it! Let's verify the removal of map operator, machan!
The removal of unused imports keeps our code clean like a well-oiled machine!
✅ Verification successful
Mind it! The map operator removal is perfectly fine, macha!
Like a perfectly timed punch scene, the removal of the 'map' operator import is spot on! Our search shows no usage of 'map' in the component, making this import as unnecessary as a villain's empty threats. Keep the code clean, that's my style!
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if 'map' operator is still used anywhere in the file
# Search for any remaining usage of map operator
rg '\bmap\s*\(' src/app/shared/components/sidemenu/sidemenu.component.ts
Length of output: 10099
Script:
#!/bin/bash
# Let's search for any usage of map operator in the sidemenu component with proper context
rg -A 3 -B 3 '\bmap\s*\(' src/app/shared/components/sidemenu/sidemenu.component.ts
Length of output: 84
src/app/shared/components/sidemenu/sidemenu.component.spec.ts (1)
Line range hint 315-319
: Mind-blowing change, thalaiva! The title update from 'Dashboard' to 'Home' looks perfect!
The modification aligns perfectly with the PR objectives, maintaining consistency across the application.
|
Clickup
https://app.clickup.com/t/86cx2whpg
Code Coverage
Please add code coverage here
UI Preview
Summary by CodeRabbit
New Features
Bug Fixes
Tests
SidemenuComponent
to verify the correct display and behavior of sidemenu options under different conditions.