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

Improve DocumentControl styling capabilities #2605

Merged

Conversation

ievgen-baida
Copy link
Contributor

@ievgen-baida ievgen-baida commented Jan 5, 2024

This PR makes it possible to adjust the following styling aspects of DocumentControl:

  • Set custom BackgroundColor of the DocumentControl;
  • Set custom TabHoverBackgroundColor and TabHoverForegroundColorfor for the tab in Hover state;
  • Set rounded corners of certain CloseCornerRadius for X Close tab button.

Changes in look/default behavior:

  • Removes forced 80% opacity of the tab in Hover state (yet retains previous behavior by default);
  • Makes Highlighted tab colors winning vs Hover colors (as this is how most of the tab controls look);
  • Adjusts the size of the close button to balance it with the size of the tab icon (16x16).

@ievgen-baida ievgen-baida force-pushed the feature/improve-document-control-styling branch from 2be82e2 to ef336a6 Compare January 18, 2024 09:47
@ievgen-baida
Copy link
Contributor Author

@cwensley, do you have any comments about the changes in this PR?

@cwensley
Copy link
Member

Hi, sorry for the delayed response! This looks great, however the close button seems to be inflated when using fixed height..

Before:
Screenshot 2024-02-10 at 1 23 25 AM

After:
Screenshot 2024-02-10 at 1 23 57 AM

Without fixed height it appears correct:
Screenshot 2024-02-10 at 1 24 18 AM

Is this intentional?

@ievgen-baida
Copy link
Contributor Author

ievgen-baida commented Feb 10, 2024

Hi, sorry for the delayed response! This looks great, however the close button seems to be inflated when using fixed height..

Before: Screenshot 2024-02-10 at 1 23 25 AM

After: Screenshot 2024-02-10 at 1 23 57 AM

Without fixed height it appears correct: Screenshot 2024-02-10 at 1 24 18 AM

Is this intentional?

Hi,
Indeed, it has been done intentionally in a separate commit (9735016).
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%).

@cwensley
Copy link
Member

Hi,
Indeed, it has been done intentionally in a separate commit (9735016).
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?

@ievgen-baida
Copy link
Contributor Author

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?

@ievgen-baida ievgen-baida force-pushed the feature/improve-document-control-styling branch from cd26a95 to d2ca4d7 Compare February 21, 2024 00:25
@ievgen-baida
Copy link
Contributor Author

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.

Signed-off-by: İevgen Baida <ievgen.baida@mendix.com>
Signed-off-by: İevgen Baida <ievgen.baida@mendix.com>
Signed-off-by: İevgen Baida <ievgen.baida@mendix.com>
Signed-off-by: İevgen Baida <ievgen.baida@mendix.com>
Signed-off-by: İevgen Baida <ievgen.baida@mendix.com>
Signed-off-by: İevgen Baida <ievgen.baida@mendix.com>
Signed-off-by: İevgen Baida <ievgen.baida@mendix.com>
@ievgen-baida ievgen-baida force-pushed the feature/improve-document-control-styling branch 2 times, most recently from 6a469af to acd2cba Compare February 23, 2024 13:50
@ievgen-baida ievgen-baida force-pushed the feature/improve-document-control-styling branch from acd2cba to 2d64d72 Compare February 23, 2024 14:52
@cwensley
Copy link
Member

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.

@cwensley cwensley merged commit dd18873 into picoe:develop Feb 26, 2024
3 checks passed
@cwensley cwensley added this to the 2.8.3 milestone Feb 26, 2024
@ievgen-baida ievgen-baida deleted the feature/improve-document-control-styling branch February 27, 2024 00:22
@ievgen-baida
Copy link
Contributor Author

Hi @cwensley, what it your plan for releasing 2.8.3 with these improvements?

@cwensley
Copy link
Member

Hey @ievgen-baida, it's released!

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

Successfully merging this pull request may close these issues.

None yet

3 participants