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: change redirect path of VfolderLazyView to current page #2731

Merged
merged 1 commit into from
Oct 2, 2024

Conversation

agatha197
Copy link
Contributor

@agatha197 agatha197 commented Oct 2, 2024

Changes:

This PR modifies the VFolderLazyView component to use the current location's pathname when navigating to a folder. Specifically:

  • Imports the useLocation hook from react-router-dom
  • Adds const location = useLocation(); to get the current location
  • Updates the onClick handler of the Typography.Link component to use location.pathname instead of the hardcoded '/data' path

Rationale:

This change allows the component to maintain the current route context when navigating to a folder, improving navigation consistency across different pages or sections of the application.

Effects:

Users will experience more predictable navigation behavior when interacting with folders, as the component will preserve the current route context instead of always redirecting to '/data'.

How to test:

  1. make a new service with mounting any folder.
  2. go to the serving page.
  3. click the folder icon of the mounted folder of service.

Checklist for reviewer:

  • Verify that the folder dialog appears while the current path is maintained

Screenshot:
image.png

Checklist:

  • Mention to the original issue
  • Documentation
  • Minium required manager version
  • Specific setting for review (eg., KB link, endpoint or how to setup)
  • Minimum requirements to check during review
  • Test case(s) to demonstrate the difference of before/after

Copy link

graphite-app bot commented Oct 2, 2024

Your org requires the Graphite merge queue for merging into main

Add the label “flow:merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “flow:hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @agatha197 and the rest of your teammates on Graphite Graphite

@github-actions github-actions bot added the size:XS ~10 LoC label Oct 2, 2024
@agatha197 agatha197 added area:ux UI / UX issue. effort:easy Need to understand only a specific region of codes (good first issue, easy). platform:general type:enhance Add new features labels Oct 2, 2024 — with Graphite App
Copy link

github-actions bot commented Oct 2, 2024

Coverage report for ./react

St.
Category Percentage Covered / Total
🔴 Statements 5.32% 340/6389
🔴 Branches 4.82% 214/4442
🔴 Functions 2.99% 63/2104
🔴 Lines 5.22% 326/6242

Test suite run success

90 tests passing in 11 suites.

Report generated by 🧪jest coverage report action from 03799df

Copy link
Member

@yomybaby yomybaby left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

graphite-app bot commented Oct 2, 2024

Merge activity

**Changes:**

This PR modifies the `VFolderLazyView` component to use the current location's pathname when navigating to a folder. Specifically:

- Imports the `useLocation` hook from `react-router-dom`
- Adds `const location = useLocation();` to get the current location
- Updates the `onClick` handler of the `Typography.Link` component to use `location.pathname` instead of the hardcoded '/data' path

**Rationale:**

This change allows the component to maintain the current route context when navigating to a folder, improving navigation consistency across different pages or sections of the application.

**Effects:**

Users will experience more predictable navigation behavior when interacting with folders, as the component will preserve the current route context instead of always redirecting to '/data'.

**How to test:**
1. make a new service with mounting any folder.
2. go to the serving page.
3. click the folder icon of the mounted folder of service.

**Checklist for reviewer:**
- [ ] Verify that the folder dialog appears while the current path is maintained

**Screenshot:**
![image.png](https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/2HueYSdFvL8pOB5mgrUQ/a8fb98ef-279e-4da7-a718-0dcec8568a5d.png)

**Checklist:**

- [ ] Mention to the original issue
- [ ] Documentation
- [ ] Minium required manager version
- [ ] Specific setting for review (eg., KB link, endpoint or how to setup)
- [ ] Minimum requirements to check during review
- [ ] Test case(s) to demonstrate the difference of before/after
@yomybaby yomybaby force-pushed the feature/change-redirect-path-of-VFolderLazyView branch from c10782e to 03799df Compare October 2, 2024 03:48
@graphite-app graphite-app bot merged commit 03799df into main Oct 2, 2024
6 checks passed
@graphite-app graphite-app bot deleted the feature/change-redirect-path-of-VFolderLazyView branch October 2, 2024 03:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:ux UI / UX issue. effort:easy Need to understand only a specific region of codes (good first issue, easy). platform:general size:XS ~10 LoC type:enhance Add new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants