Skip to content

Conversation

austincondiff
Copy link
Collaborator

@austincondiff austincondiff commented Mar 5, 2024

Description

The new Navigation Style setting allows users to open a single file instead of multiple files in an editor. When this is set, the tab bar is hidden when there is only a single temporary tab.

When splitting an editor, the new editors only tab (copied from the previous editor) will now be a temporary tab (previously not temporary). This is so that when using a "open in place" navigation style, the tabs will remain hidden when splitting an editor.

If going back or forward in history to a file for a tab that was previously closed, we now open it as a temporary tab. Again this will prevent the tab bar from unnecessarily showing up in certain edge-cases.

We are also now invalidating history when opening file with project navigator so user cannot go forward.

Related Issues

Checklist

  • Add Navigation Style setting
  • Hide tab bar if only one temporary tab exists in an editor when navigation style is set to open in place
  • Add navigation controls to path bar when tab bar is hidden
  • Adjust editor top safe area 1
  • Prevent double clicking file to make it a permanent tab
  • Prevent file edit or save from making the tab permanent
  • If only one tab in an editor exists when changing Navigation Style to Open in Place, convert it to a temporary tab
  • If any open editor splits contain a non-temporary tab, show the tab bar for all editors. 2
  • When splitting an editor, open the new editors only tab as a temporary tab
  • Navigating through the file history to a previously closed tab should reopen it as a temporary tab
  • Invalidate history when opening file with project navigator so user cannot go forward

  • I read and understood the contributing guide as well as the code of conduct
  • The issues this PR addresses are related to each other
  • My changes generate no new warnings
  • My code builds and runs on my machine
  • My changes are all related to the related issue above
  • I documented my code

Screenshots

Screen.Recording.2024-03-05.at.9.23.14.AM.mp4

Footnotes

  1. CodeEditTextView will need work to fix a bug revealed by this work, see below video (@thecoolwinter)
    https://github.com/CodeEditApp/CodeEdit/assets/806104/f1a57323-98a7-4bdb-baaa-aa39ddf4e166

  2. I might need to iterate over all editor splits to determine this. See TODO in code. (@Wouter01 any ideas?)

@Wouter01
Copy link
Member

Wouter01 commented Mar 5, 2024

Is this issue related to the safe area height?

@austincondiff
Copy link
Collaborator Author

I had to modify the safe area height depending on what is in display. CodeEditTextView does not update when the safe area is changed. Also I discovered that the line numbers are hidden in the safe area and need to be visible.

@Wouter01 The footnote I mentioned you in was relating to the EditorManager (previously called the TabGroupManager). We need to iterate over all editors to see if any of the editors have temporary tabs. If any do, we need to show the tab bar in all editors when the navigation style is set to open in place.

@Wouter01
Copy link
Member

Wouter01 commented Mar 5, 2024

The code mentioned with the TODO seems fine to me! It's been some time since I worked on it so I can't tell for sure, but I don't see anything wrong with it immediately

@austincondiff austincondiff marked this pull request as ready for review March 6, 2024 08:13
@austincondiff
Copy link
Collaborator Author

This is now ready for review.

0xWDG
0xWDG previously approved these changes Mar 6, 2024
Copy link
Collaborator

@0xWDG 0xWDG left a comment

Choose a reason for hiding this comment

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

LGTM,

One note, maybe it is better to change
.padding(.horizontal, shouldShowTabBar ? file == nil ? 10 : 4 : 0)
to
.padding(.horizontal, shouldShowTabBar ? (file == nil ? 10 : 4) : 0)
to make it more clear what the ternary operator is doing.
But that's more my personal style preference.

@austincondiff
Copy link
Collaborator Author

@FastestMolasses and @0xWDG I just pushed both of your suggestions.

@austincondiff austincondiff added enhancement New feature or request settings labels Mar 7, 2024
0xWDG
0xWDG previously approved these changes Mar 8, 2024
Copy link
Collaborator

@0xWDG 0xWDG left a comment

Choose a reason for hiding this comment

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

🎉

@thecoolwinter
Copy link
Collaborator

I have a PR open to fix the related issue on CodeEditSourceEditor here.

austincondiff pushed a commit to CodeEditApp/CodeEditSourceEditor that referenced this pull request Mar 9, 2024
### Description

- Refactors a few style functions to correctly update the gutter view's
position when the content insets change.
- Adds the gutter view position to the content insets test case.

### Related Issues

* Required to finish
[CodeEdit#1603](CodeEditApp/CodeEdit#1603)

### Checklist

- [x] I read and understood the [contributing
guide](https://github.com/CodeEditApp/CodeEdit/blob/main/CONTRIBUTING.md)
as well as the [code of
conduct](https://github.com/CodeEditApp/CodeEdit/blob/main/CODE_OF_CONDUCT.md)
- [x] The issues this PR addresses are related to each other
- [x] My changes generate no new warnings
- [x] My code builds and runs on my machine
- [x] My changes are all related to the related issue above
- [x] I documented my code

### Screenshots

N/A
@austincondiff
Copy link
Collaborator Author

austincondiff commented Mar 9, 2024

@thecoolwinter merged now. Do you want to create a new release of CESE and I can add the new version in this branch?

…for a teb that was previously closed, we now open it as a temporary tab
@austincondiff austincondiff changed the title Navigation style setting Navigation style setting and history improvements Mar 11, 2024
@austincondiff austincondiff changed the title Navigation style setting and history improvements Navigation style setting and file history navigation improvements Mar 11, 2024
@austincondiff austincondiff marked this pull request as ready for review March 11, 2024 19:54
@thecoolwinter
Copy link
Collaborator

@austincondiff thanks for the approval on CESE, I made a release for that package if you update to 0.7.2 it should work correctly.

@austincondiff austincondiff dismissed FastestMolasses’s stale review March 11, 2024 20:14

Made suggested changes

bombardier200
bombardier200 previously approved these changes Mar 12, 2024
@austincondiff
Copy link
Collaborator Author

Please note the following additions:

When splitting an editor, the new editors only tab (copied from the previous editor) will now be a temporary tab (previously not temporary). This is so that when using a "open in place" navigation style, the tabs will remain hidden when splitting an editor.

If going back or forward in history to a file for a tab that was previously closed, we now open it as a temporary tab. Again this will prevent the tab bar from unnecessarily showing up in certain edge-cases.

We are also now invalidating history when opening file with project navigator so user cannot go forward.

@austincondiff austincondiff self-assigned this Mar 12, 2024
Copy link
Collaborator

@thecoolwinter thecoolwinter left a comment

Choose a reason for hiding this comment

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

Some UX stuff and looks like @matthijseikelenboom comment wasn't resolved.

@austincondiff austincondiff enabled auto-merge (squash) March 12, 2024 15:35
@austincondiff austincondiff disabled auto-merge March 12, 2024 15:35
@austincondiff austincondiff merged commit 4d9a5d4 into CodeEditApp:main Mar 12, 2024
Copy link
Contributor

@matthijseikelenboom matthijseikelenboom left a comment

Choose a reason for hiding this comment

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

I see it's already merged, but still

}

/// Flattens the splitviews.
func flatten() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Calling this function "flatten" is a bit weird. editorManager.flatten() kind of reads like this function flattens the manager

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Probably should be flattenEditors. I did not write this. We can go in and change this later.

Copy link
Collaborator Author

@austincondiff austincondiff Mar 12, 2024

Choose a reason for hiding this comment

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

Come to think of it, we might even consider other words like join, combine, or merge. (VS Code uses join)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request settings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

✨ Navigation Style Setting in Navigation Settings
7 participants