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: can dock the monitor widget to the "right" #2102

Merged
merged 1 commit into from
Jul 13, 2023

Conversation

dankeboy36
Copy link
Contributor

@dankeboy36 dankeboy36 commented Jun 17, 2023

Motivation

This is a quality of life improvement until the Serial Monitor widget is not detachable from the IDE2 window: arduino/arduino-ide#289. This is the simplest solution to use larger screens more efficiently. One can keep the widget at the "bottom", or move it to the "right".

monitor.DockArea.mp4

Change description

Added a new preference (arduino.monitor.dockPanel) to specify the area of the application shell where the Serial Monitor widget resides. The possible values are "bottom" and "right". The default value is "bottom".

The dock area is per application and not per workspace or window. However, advanced users can create the ./.vscode/settings.json and configure it per sketch preference.

Other information

Related: #2075 (comment)

Reviewer checklist

  • PR addresses a single concern.
  • The PR has no duplicates (please search among the Pull Requests before creating one)
  • PR title and description are properly filled.
  • Docs have been added / updated (for bug fixes / features)

@kittaakos kittaakos added type: enhancement Proposed improvement topic: code Related to content of the project itself topic: serial monitor Related to the Serial Monitor labels Jun 17, 2023
@kittaakos kittaakos self-requested a review June 17, 2023 11:20
Copy link
Contributor

@kittaakos kittaakos left a comment

Choose a reason for hiding this comment

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

monitor.widgetDockArea?

Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

I'm wondering about the term "area". I have struggled to understand the correct terminology to use when referring to Arduino IDE UI components.

Here we have the terms "Side Bar" for the UI containers on the left and right side of the window and and "Panel" for the container at the bottom:

Here we have the terms "area" and "panel" used for all three containers (but evidently not for the "main area"?):

https://www.typefox.io/blog/flexible-window-layout-in-theia-ide

I don't like how VS code uses a different term for a container depending on its location. I think it is more intuitive (especially in the context of Theia's "flexible window layout") to use the same term for all containers. I find "area" to be overly ambiguous, so I have been using the term "panel":

  • "main panel"
  • "bottom panel"
  • "left panel"
  • "right panel"

@per1234
Copy link
Contributor

per1234 commented Jul 3, 2023

I've thought about it more and I think that in general it is best to use the terminology established by VS Code since this is by far a more significant precedent and more likely to already be familiar to the users than the alternate terminology established (inconsistently) in an obscure blog post.

However, it is not possible to follow the VS Code terminology exactly in this case because here we need a single term that can apply to both the "Panel" and the "Secondary Sidebar". Obviously the term "sidebar" can't be used for that purpose, so we are left with "panel".

So I suggest renaming the setting ID arduino.monitor.dockPanel. This will cause the setting name shown in the GUI advanced settings interface to be "Arduino > Monitor: Dock Panel" instead of the current "Arduino > Monitor: Dock Area"

image

The existing "bottom" and "right" setting values will work fine in that context. I think the setting description is OK as it is now.

@per1234
Copy link
Contributor

per1234 commented Jul 5, 2023

@dankeboy36 is the PR ready for another review round?

@dankeboy36
Copy link
Contributor Author

PR ready for another review round?

Yes. Please take another look at the changes. The new preference key is now arduino.monitor.dockPanel. I have updated the PR description too. Thank you!

@kittaakos kittaakos self-assigned this Jul 5, 2023
@@ -258,6 +266,17 @@ export const ArduinoConfigSchema: PreferenceSchema = {
),
default: undefined,
},
'arduino.monitor.dockArea': {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'arduino.monitor.dockArea': {
'arduino.monitor.dockPanel': {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Excellent catch. The compiler should find such errors, so I added a tiny TypeScript trick to let the compiler error when there is a typo next time.

Added a new preference (`arduino.monitor.dockPanel`) to specify the
location of the application shell where the _Serial Monitor_ widget
resides. The possible values are `"bottom"` and `"right"`. The default\
value is the `"bottom"`.

The dock panel is per application and not per workspace or window.
However, advanced users can create the `./.vscode/settings.json` and
configure per sketch preference.

Signed-off-by: dankeboy36 <[email protected]>
Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

Thanks @dankeboy36!

@kittaakos kittaakos merged commit f6a4325 into arduino:main Jul 13, 2023
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself topic: serial monitor Related to the Serial Monitor type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants