-
Notifications
You must be signed in to change notification settings - Fork 1
Add option to open multiple tabs (Resolves #23) #24
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
base: main
Are you sure you want to change the base?
Conversation
|
Thanks for taking this on. I should probably build us a tab widget in Lucid that we can pull in here. For the record, I really dislike:
We should make these things configurable as soon as possible. |
| void didUpdateWidget(covariant CodeLines oldWidget) { | ||
| super.didUpdateWidget(oldWidget); | ||
|
|
||
| if (widget.codeLines.length != oldWidget.codeLines.length) { |
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.
Is this the correct condition? Why can't the line content change while retaining the same number of lines?
Or, if these keys just need to exist one per line, why are we clearing them? Why wouldn't we add/remove the necessary number?
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.
Updated. Also, I think it makes more sense to use a list of GlobalKeys instead of a map, because now we will always have the correct number of keys, i.e., we won't need to create them on demand.
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| super.build(context); |
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.
What is this?
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.
It's part of the AutomaticKeepAliveClientMixin, to keep the state while changing between tabs.
example/lib/ide/ide_controller.dart
Outdated
|
|
||
| import 'package:flutter/foundation.dart'; | ||
|
|
||
| /// Provides the IDE functionallity, like opening and editing files. |
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.
If this is going to be called an IdeController, which is probably the most expansive concept that could exist, it needs to have a much clearer mission than "Provides the IDE functionality".
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.
I tried to make the docs more specific to what we have now, but we might add more responsibilities to this controller.
example/lib/ide/ide_controller.dart
Outdated
| /// If there is no active editor, opens a new editor. | ||
| /// | ||
| /// If the file is already open in another editor, shows the existing editor. | ||
| Future<void> openFileAtActiveEditor(String fileUri) async { |
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.
We can try this level of specificity for now, but my gut feeling is that this is too much of a micromanagement on the part of this globally relevant controller.
Also, if we're going to have an enum that understands editor tab options, shouldn't we have a single method to open a file and have that method take the enum?
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.
Modified to expose a single method in the public API.
example/lib/ide/ide_controller.dart
Outdated
| await _openFile(fileUri, _editorData.value.openEditors.length); | ||
| } | ||
|
|
||
| /// Shows the existing editor for the file with the given URI, if any. |
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.
Please try to aim the explanation at the questions the reader is likely to have. In this case, the obvious question would be "where did the editor go? why would I need to 'show' it? what does that mean?"
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.
Updated.
Add option to open multiple tabs (Resolves #23)
It's common for IDE's to allow opening multiple files in tabs. This PR adds the initial ability to do that in inception. It does not add the functionality to open side-by-side tabs, but it's something to keep in mind.
In this PR, a single click opens a file in the active tab, and a double click opens a file in a new tab. This can be changed later.
This PR introduces a
IdeControllerclass to hold the state of the IDE. For now it only holds the open file editors, but I think that when we implement editable files we can use this class to dispatch edit commands, like inserting text, renaming files, saving files, etc.Screen.Recording.2025-02-15.at.16.04.26.mov