-
Notifications
You must be signed in to change notification settings - Fork 699
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
Fixes #3370. Adds visual highlight for press and hold (for Button
, Checkbox
, RadioGroup
, and Border
)
#3372
Conversation
@BDisp give this PR a try. Bring it down locally and note how i've implemented a UI effect on Button that shows when it's been pressed/released. Note that if you do a mouse button press on a button, and then move off of it, it does NOT click, but if you move it back (while still holding the button) down and then release, it does click. I think this is what you want:
...AND is what I want:
|
Yes that fit what I said. It isn't clicking in the same point that matters but in the same object. That's good. |
I think this means removing the I just did a quick test with this: And Button works perfectly (I think). |
Also, I'd love your thoughts on the "repeat accelaration" I put in. See how the Accept Count accelerates after 2 seconds of holding? Terminal.Gui/Terminal.Gui/ConsoleDrivers/WindowsDriver.cs Lines 1711 to 1748 in 1cca002
Ideally this would be configurable, and I don't like the coupling between |
…would benefit. Note this breaks Charmap because ScrollView blindly grabs mouse on Enter. This will be fixed in gui-cs#3323.
Works, but not optimally. The entire view invert. What would be really nice is if just the radio item under the mouse inverted. We can fix this later.
Do this issue is happening to you? Click on the WindowsTerminal_DQs6FVVFAA.mp4 |
That also depends of what you are really doing. If you are dragging a view and want always highlighted until releasing it, that ok. But if you are generating a animation of a button press then if the mouse is outside of his bounds then the color is restored to alert that the button isn't selected if the mouse button is released. You also need to decide if you pressing and holding on a button and move the mouse to another button what the effect you want. Restores the previous and highlight the new button? |
- Started MouseEventEventArgs -> MouseEvent transition - for OnMouseEvent. Partially fixes gui-cs#3029. - Refactored Appliation.OnMouseEvent and View.OnMouseEvent to match design guidelines. - Re-impleented highlight, enabling extensibility (see Border). - Beefed up unit tests
@BDisp - huge update here. Please take a look! |
I now like more the buttons behaviors. The button only get focus if the mouse button is pressed. If I release the mouse button on another button the clicked event isn't processed which I like very much. One animation you maybe can do is only when moving the mouse without pressing any mouse button is highlight the button when the mouse is over it with the focus color and restore the previous color when the mouse is out of it. devenv_WpuVIZcfyb.mp4 |
This is weird. If the window cannot move, it must be prevented from moving. WindowsTerminal_I6opvFlJk8.mp4 |
This code work but you have to override the private bool _isMouseEnter;
/// <inheritdoc />
protected internal override bool OnMouseEnter (MouseEvent mouseEvent)
{
if (mouseEvent.Flags == MouseFlags.ReportMousePosition)
{
_isMouseEnter = true;
SetNeedsDisplay ();
}
return base.OnMouseEnter (mouseEvent);
}
/// <inheritdoc />
protected internal override bool OnMouseLeave (MouseEvent mouseEvent)
{
if (mouseEvent.Flags == MouseFlags.ReportMousePosition)
{
_isMouseEnter = false;
SetNeedsDisplay ();
}
return base.OnMouseLeave (mouseEvent);
}
/// <inheritdoc />
public override void OnDrawContent (Rectangle contentArea)
{
TextFormatter?.Draw (
BoundsToScreen (contentArea),
HasFocus || (_isMouseEnter && CanFocus && !HasFocus) ? GetFocusColor () : GetNormalColor (),
HasFocus ? ColorScheme.HotFocus : GetHotNormalColor (),
Rectangle.Empty
);
} WindowsTerminal_6GaJHEo6os.mp4 |
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.
LGTM.
// Only delay if there's not a view wanting continuous button presses | ||
if (Application.WantContinuousButtonPressedView is null) |
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.
This isn't necessary because this async method is only called if the _isButtonDoubleClicked or _isOneFingerDoubleClicked
is true
. So, even a view which WantContinuousButtonPressed
is true
may want a ButtonTripleClicked
event. Another detail, for a click, double click or triple click event occurs, it means that were also button released each time, which will generate the clicked events.
Am I missing something?
// QUESTION: Why 300ms? | ||
await Task.Delay (300); |
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.
In the PR #3386 I decreased to 200 to not wait so long.
Added Color.GetHighlightColor. Simple implementation for now. Needs to be more robust.
Button
Button
, Checkbox
, RadioGroup
, and Border
)
This feature is a nice touch. 👍 |
A thought while I'm waiting for my PC to update and boot that this brought up. And maybe it's already there, but I can't test on my phone... Is behavior like many graphical apps have, where pressing and holding alt will highlight or underline all the hotkeys of elements on screen, if not already displayed (including bindings, potentially) already in there or easy to bolt onto this? 🤔 I mean yeah of course a consumer could do it themselves but it's a nice feature to have in a UI library. If not there already, maybe a vNext feature? |
Or... Thinking about things that are strictly features and not core framework API... Have we ever considered having a separate "toolkit" library for features as seems to be pretty common in various libraries these days? Basically the modular approach, that even .net itself now uses. 🤷♂️ Just brainstorming. |
That approach is still easy to have an all-in-one with, as well. You just include all the modules in an all in one library project and publish single file and 💥: there's your all-in-one. |
Sadly, getting just I am excited for #2612 though! |
Figures 😆 And yes, I love the improvements! Feels like things are getting shored up nicely. 🙂 |
Fixes
Button
support repeat via continuous presses #3370Proposed Changes/Todos
Color.GetHighlightColor
increases brightnessView.OnMouseEvent
andApplication.OnMouseEvent
Buttons
scenario to demonstrateView
and addView.InvertColorsOnPress
propertyCheckbox
,Button
,DatePicker
, andColorPicker
to utilize.RadioGroup
to utilize - partial - entire view inverts, not just the radio item under the mouseBorder
for view dragNetDriver
andCursesDriver
- Dependent on Not always gettingButton1Clicked
events onNetdriver
orCursesDriver
#3374 being fixed (Fixes #3374 -Curses
andNet
drivers don't generate click events unless mouse has not moved #3375).Pull Request checklist:
CTRL-K-D
to automatically reformat your files before committing.dotnet test
before commit///
style comments)