Closed ievgen-baida closed 7 months ago
@cwensley, do you have any comments about the changes in this PR?
Hi, sorry for the delayed response! This looks great, however the close button seems to be inflated when using fixed height..
Before:
After:
Without fixed height it appears correct:
Is this intentional?
Hi, sorry for the delayed response! This looks great, however the close button seems to be inflated when using fixed height..
Before:
After:
Without fixed height it appears correct:
Is this intentional?
Hi, Indeed, it has been done intentionally in a separate commit (9735016263316d2de3fb370f9e87e16c334792db). I have added description of what has been done in the description of the PR as well.
In short, we needed to balance the size of the close button and the X with the size of the icon, otherwise it appears too small using pre-existing logic (33%/33%/33%).
Hi, Indeed, it has been done intentionally in a separate commit (https://github.com/picoe/Eto/commit/9735016263316d2de3fb370f9e87e16c334792db). I have added description of what has been done in the description of the PR as well.
In short, we needed to balance the size of the close button and the X with the size of the icon, otherwise it appears too small using pre-existing logic (33%/33%/33%).
I'm not sure having different size based on fixed height vs not is ideal here, it should behave/look the same regardless imo. Also I'm not a huge fan of the size of the X personally with the new logic, but I understand this is entirely subjective. Maybe we need to have a better default scale or logic based on its size?
I'm not sure having different size based on fixed height vs not is ideal here, it should behave/look the same regardless imo. Also I'm not a huge fan of the size of the X personally with the new logic, but I understand this is entirely subjective. Maybe we need to have a better default scale or logic based on its size?
That is another way of doing it.
However, it would introduce potentially unwanted visual differences for people currently using DocumentControl
without specifying newly introduced UseFixedTabHeight
. It basically switches DocumentControl
into the mode where the default size of the icon and the X is used instead of relying on the actual images' sizes.
The current default logic allows to use any images, but it does not allow to use higher-resolution images and make use of scaling when rendered on high-DPI screens.
What kind of logic do you have in mind?
Hi @cwensley,
I have implemented an unconditional logic that does not use custom values for margin or size of the close button.
Please have a look.
Hey @ievgen-baida, thanks for the updates! This looks great. If anything that scale could be added as another property that can be configured, but at this point just having a consistent size is perfect.
Hi @cwensley, what it your plan for releasing 2.8.3 with these improvements?
Hey @ievgen-baida, it's released!
This PR makes it possible to adjust the following styling aspects of
DocumentControl
:BackgroundColor
of theDocumentControl
;TabHoverBackgroundColor
andTabHoverForegroundColorfor
for the tab in Hover state;CloseCornerRadius
forX
Close tab button.Changes in look/default behavior:
Highlighted
tab colors winning vsHover
colors (as this is how most of the tab controls look);