Skip to content

Shorten editor tab file paths #2383

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

Merged
merged 8 commits into from
Mar 24, 2023
Merged

Shorten editor tab file paths #2383

merged 8 commits into from
Mar 24, 2023

Conversation

ianyong
Copy link
Member

@ianyong ianyong commented Mar 23, 2023

Description

When displaying file paths in the editor tab, show the shortest possible file path that uniquely identifies each tab among all open tabs. This is similar to how most code editors do it.

image

Part of #2176.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Code quality improvements

How to test

The multiple files mode is not yet ready for production and is disabled programmatically. Enable multiple files mode for testing by changing the condition to false:

const toggleMultipleFilesModeButton = React.useMemo(() => {
// TODO: Remove this once the multiple file mode is ready for production.
if (true) {
return <></>;
}
return (
<ControlBarToggleMultipleFilesModeButton
isMultipleFilesEnabled={isMultipleFilesEnabled}
toggleMultipleFilesMode={() => dispatch(toggleMultipleFilesMode(workspaceLocation))}
/>
);
}, [dispatch, isMultipleFilesEnabled, workspaceLocation]);

@ianyong ianyong requested a review from zhaojj2209 March 23, 2023 17:09
@coveralls
Copy link

coveralls commented Mar 23, 2023

Pull Request Test Coverage Report for Build 4508193665

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 18 of 21 (85.71%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.07%) to 34.881%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/commons/editor/tabs/EditorTabContainer.tsx 0 3 0.0%
Files with Coverage Reduction New Missed Lines %
src/commons/editor/tabs/EditorTabContainer.tsx 1 10.0%
Totals Coverage Status
Change from base Build 4508186654: 0.07%
Covered Lines: 4691
Relevant Lines: 12533

💛 - Coveralls

Copy link
Contributor

@zhaojj2209 zhaojj2209 left a comment

Choose a reason for hiding this comment

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

LGTM

@ianyong ianyong enabled auto-merge (squash) March 24, 2023 06:05
@ianyong ianyong disabled auto-merge March 24, 2023 06:06
We need to pass in an empty string instead of '/' because otherwise,
the first slash of file paths would be removed and the result would
not be file paths.
@ianyong ianyong enabled auto-merge (squash) March 24, 2023 06:10
@ianyong ianyong merged commit 93ec868 into master Mar 24, 2023
@ianyong ianyong deleted the shorten-editor-tab-file-paths branch March 24, 2023 06:18
RichDom2185 pushed a commit to NUS-CS1101S/cadet-frontend that referenced this pull request Jul 15, 2023
* Shorten editor tab file paths

* Remove empty file path segments

* Add test cases for getting the shortened unique file paths

* Exclude the workspace base path from being displayed in editor tab

* Add comments & better name variables in getShortestUniqueFilePaths

* Fix value of default base file path

We need to pass in an empty string instead of '/' because otherwise,
the first slash of file paths would be removed and the result would
not be file paths.
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.

3 participants