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

Why ViewMouse doesn't handle the movable procedure instead of the Adornment? #3342

Closed
BDisp opened this issue Mar 20, 2024 · 11 comments
Closed
Labels

Comments

@BDisp
Copy link
Collaborator

BDisp commented Mar 20, 2024

Before it was possible to move a Toplevel without Border or a Window with/out Border. Now only Adornment can handle this, which in my opinion isn't the right behavior. Should be the View itself responsible to handle this based on the Arrangement setting.
So the logic on the OnMouseEvent in the Adornment should be on the OnMouseEvent of the ViewMouse.cs file.

@tig
Copy link
Collaborator

tig commented Mar 20, 2024

What is the end-user use case you are considering here?

@BDisp
Copy link
Collaborator Author

BDisp commented Mar 20, 2024

What is the end-user use case you are considering here?

In the BackgroundWorker scenario I use a WorkerApp which is a Toplevel without Border to proof the moveable is also possible. Now it can't move. I already have a solution for this and I'll submit a PR. Maybe you agree with the change that is only 2 or 3 lines.

Another end-user is the possibility of the designer can move the views more easily without adding Border. What must avoiding this is the Arrangement setting.

@BDisp
Copy link
Collaborator Author

BDisp commented Mar 20, 2024

Is there any reason why the Padding isn't allowing to move the view if it's the only adornment? It overrides the OnMouseEvent to only set focus to the parent. I don't want to change this if it has a special meaning for you.
I think that if it doesn't have any subviews will be possible to move the view if it's moveable.

@BDisp
Copy link
Collaborator Author

BDisp commented Mar 20, 2024

Toplevel created by the Application.Init shouldn't be moveable, otherwise it will behave badly because it represent all the available space on a terminal.

@tig
Copy link
Collaborator

tig commented Mar 20, 2024

Is there any reason why the Padding isn't allowing to move the view if it's the only adornment? It overrides the OnMouseEvent to only set focus to the parent. I don't want to change this if it has a special meaning for you.

I think that if it doesn't have any subviews will be possible to move the view if it's moveable.

I actually think it's debatable whether margin should support drag.

It's all debatable.

I decided it made no sense for padding.

@BDisp
Copy link
Collaborator Author

BDisp commented Mar 21, 2024

I actually think it's debatable whether margin should support drag.

It's all debatable.

I decided it made no sense for padding.

Sure, but I only referring if the Padding it's the only Adornment. In this case I think clicking in the Padding is possible to move the view, otherwise a view with only a Padding never can move the view if it's moveable. I think it's the right procedure, but I could be wrong.

@tig
Copy link
Collaborator

tig commented Mar 21, 2024

Toplevel created by the Application.Init shouldn't be moveable, otherwise it will behave badly because it represent all the available space on a terminal.

Right. Another reason Toplevel as a class was a regrettable design choice.

With 'Arrangment' it's an easy fix.

@tig
Copy link
Collaborator

tig commented Mar 21, 2024

Is there any reason why the Padding isn't allowing to move the view if it's the only adornment? It overrides the OnMouseEvent to only set focus to the parent. I don't want to change this if it has a special meaning for you.

I think that if it doesn't have any subviews will be possible to move the view if it's moveable.

I actually think it's debatable whether margin should support drag.

It's all debatable.

I decided it made no sense for padding.

More context:

Margin and Padding are typically invisible to the user. Border typically, literally, shows a border. I believe users will be surprised if they click and drag on something invisible and it starts dragging.

See "the principle of least surprise"

@BDisp
Copy link
Collaborator Author

BDisp commented Mar 21, 2024

More context:

Margin and Padding are typically invisible to the user. Border typically, literally, shows a border. I believe users will be surprised if they click and drag on something invisible and it starts dragging.

Margin is always invisible unless the effect3D is implemented. Padding could ne not always invisible but could have subviews.
Border is really the correct class where the OnMouseEvent should handle the moveable view.
Maybe the only exception is in the case where the view has no adornments and the color is highlighted from the background color and so the user has a visible area to visualize the movement effect.

See "the principle of least surprise"

I already saw, thanks. I suggest only the Border to be responsible to move the view. The exception is on views without adornments which the user want make it visible from the superview background color and want move it. If the view has Margin and Border the mouse must be over the Border thickness to could move the view. What do you think?

@tig
Copy link
Collaborator

tig commented Mar 21, 2024

I concur regarding Border: I'll change things so only Border handles move/size asap.

However, I do not think the ability to drag a view with no Border should be built-in. Here's why:

  • It's not discoverable. Yes, we could invent a way of making such a view indicate it can be moved/dragged visually, but that's just more complexity.
  • There isn't a keyboard interface (that's visible). With Add mouse/keyboard resizing of views #2537 we'll have the ability to support keyboard move/resize.
  • It's very rare use-case.
  • Just add a border to the view. It could be with just Thickness (0, 1, 0, 0) and with Title = string.Empty, for example*.

If this Issue is not fixed, there's an alternative if someone wants a non-border view to be movable with the mouse:

  • Just use the MouseEvent event, or override OnMouseEvent in the view (in your case WorkerApp) to handle the move/drag themselves.

For this reason, I'd like to close this issue as "by design'.

*Regarding Title visibiility: I am not happy with the way the Border currently determines whether to draw/not draw the Title. I plan on adding some sort of setting that enables adjusting the border "style". Once I get the time to upgrade Border to be based on a set of subviews, the Title will be rendered as a Label. Thus, view.Border.TitleLabel.Visible = false will turn off the Title.

@tig
Copy link
Collaborator

tig commented Mar 21, 2024

For example this completely addresses your desired UI using even less screen real estate (instead of using a Label to duplicate "TItle", just use Title):

ekQwBPw 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants