-
-
Notifications
You must be signed in to change notification settings - Fork 335
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
Add capability for styling DocumentControl tabs #2574
Add capability for styling DocumentControl tabs #2574
Conversation
c77e57e
to
d0cc489
Compare
@@ -417,8 +584,11 @@ void Drawable_Paint(object sender, PaintEventArgs e) | |||
|
|||
posx = 0; | |||
|
|||
DrawTab(g, tabPrev, -1); | |||
DrawTab(g, tabNext, -1); | |||
if (allowNavigating) |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
d0cc489
to
c4bb3ff
Compare
Hey @eng-myousif, thanks a ton for the PR! This looks great. A few things, first I don't think the color properties should be on the Additionally, it would be great to see the As for the name of |
g.DrawImage(tab.Image, tabRect.X + TabPadding.Left + (maxImageSize.Width - imageSize.Width) / 2, (tabDrawable.Height - imageSize.Height) / 2); | ||
g.SaveTransform(); | ||
g.ImageInterpolation = ImageInterpolation.High; | ||
var imageSize = tab.ImageSize.IsEmpty ? tab.Image.Size : tab.ImageSize; |
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.
Having ImageSize
public property and setting it to a deliberately "wrong" value can end up in less-than-beautiful results. I would argue the desired image size should be lead by the ThemedDocumentControl
itself (it knows the dimensions of the tabs it renders and it also should know the ideal dimensions for the image inside that tab).
Shall we just scale tab.Image
up/down to fit that ideal rectangle?
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 got your point, I will revert changes for exposing ImageSize and let you know.
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.
Hi @cwensley ,
-
I moved the color properties to
ThemedDocumentControlHandler
-
Implemented
PageClosing
for the Gtk handler. -
Couldn't implement
HasNavigationButtons
(now calledHasNavigationButtons
) for the Gtk handler as the gtk Nootbook control doesn't allow to hide theHasForwardStepper
andHasBackwordStepper
because they are readonly, Hence I move the property to theThemedDocumentControlHandler
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.
Hi @ievgen-baida ,
I have introduce a property UseFixedTabHeight
along with a checkbox in the DocumentControlSection demo dialog. When set UseFixedTabHeight
to true it resizes the images inside the Tab height, then we have a fixed tab height that is calculated by the font size and vertical padding.
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.
Looks good to me.
Only thing, I wonder if instead of relying on the font size when calculating the height of the image we would rather use some sensible standard size, like 16x16?
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.
@ievgen-baida, sounds like a good idea. Applied.
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.
Thank you. Looks good to me.
5463328
to
6389fe0
Compare
e99d2eb
to
2b57282
Compare
Hey @eng-myousif, this looks great! Let me know when it's ready and I'll merge in the PR. |
Hi @cwensley, we finalized it from our side. You can merge it. Would it be possible to create a patch release, so that we could use it in production? |
Hey @ievgen-baida, yes I can certainly create a patch release. I'll try to do that shortly. |
This PR is aiming to expose some feature for
DocumentControl
the following:1- It is possible to set the Background and Foreground colors for Tabs and Close (
x
) button.2- It is possible to cancel a closing a page when attempt to close it using close (
x
) button.