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

integrate groupbox control #3913

Closed
wants to merge 8 commits into from

Conversation

mameolan
Copy link

@mameolan mameolan commented May 9, 2020

What does the pull request do?

adds the missing GroupBox Control

Checklist

add sample page
@dnfclas
Copy link

dnfclas commented May 9, 2020

CLA assistant check
All CLA requirements met.

Copy link
Member

@danwalmsley danwalmsley left a comment

Choose a reason for hiding this comment

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

Very good, just 1 potential issue found

src/Avalonia.Themes.Default/GroupBox.xaml Show resolved Hide resolved
mameolan and others added 2 commits May 10, 2020 07:46
Copy link
Member

@danwalmsley danwalmsley left a comment

Choose a reason for hiding this comment

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

1 last minor change

samples/ControlCatalog/MainView.xaml Outdated Show resolved Hide resolved
@mameolan mameolan requested a review from danwalmsley May 11, 2020 18:45
@danwalmsley
Copy link
Member

@mameolan sorry about this, but any change we can have it styled like the wpf one?

image

@JamRemco
Copy link
Contributor

@mameolan sorry about this, but any change we can have it styled like the wpf one?

image

Seiously that's so win95 haha ;)

@Gillibald
Copy link
Contributor

I personally dislike this control because it brings no functionality. It is just a styled HeaderedContentControl

@grokys
Copy link
Member

grokys commented May 12, 2020

For reference, this control currently looks like this:

image

can we have it styled like the wpf one?

I don't think this would be a good idea, the WPF one is Win9x styled and we use a more UWP-style. Thing is, the GroupBox doesn't really have a more modern style, it's a control that doesn't really exist any more. Can anyone show me an example of a GroupBox in a modern application?

I personally dislike this control because it brings no functionality. It is just a styled HeaderedContentControl

Yeah, for this reason and the styling/outdatedness issues, I'm not sure we would want to accept this control into Avalonia core.

@danwalmsley
Copy link
Member

@grokys the argument for having it could be a lot of wpf legacy code might have it, and it just makes it easier when copying and pasting examples... i remember now this is the second time someone has sent a PR for this...

im neutral on having it though.

@kekekeks
Copy link
Member

Could we have a proper template for HeaderedContentControl and keep GroupBox as a deprecated alias?

@JamRemco
Copy link
Contributor

JamRemco commented May 22, 2020

@grokys
I got you point, its like template a textblock and call it a new control. however we could make a Styleclass for HeaderContentControl thats kinda looks and act like a groupbox.
So instead of a new control you just got a style
xaml <HeaderedContentControl classes = "Groupnox"/>
That way Everybody's happy.

@danwalmsley agreed people seems to like/use it .

@kekekeks how would a "proper" template looks/function in your oppinion.

@kekekeks
Copy link
Member

@JamRemco Right now HeaderedContentControl has no template at all.

@danwalmsley
Copy link
Member

will close for now... @mameolan sorry... hope you will continue to contribute :)

@CorruptComputer
Copy link

I actually am a fan of how these look, especially when making small UI's to command line apps. Is there no chance that this could get added still? I don't seem the harm with it, since if you don't like it you can just not use it.

@rstm-sf
Copy link
Contributor

rstm-sf commented Aug 12, 2020

You can create a separate control, the source code is available ;)

Also it looks like it is available in this project https://github.com/mameolan/Avalonia.ExtendedToolkit

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

Successfully merging this pull request may close these issues.

9 participants