Add outline to a label (2) #22803

ildar170975 · 2024-11-13T04:59:22Z

Breaking change

Proposed change

Follow up #22392

Before:

After:

Before:

After:

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New feature (thank you!)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Example configuration

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue or discussion:
Link to documentation pull request:

Checklist

The code change is tested and works locally.
There is no commented out code in this PR.
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

coderabbitai · 2024-11-13T04:59:28Z

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

MindFreeze

At this point let's just add the outline in the ha-label component

ildar170975 · 2024-11-13T07:36:01Z

in the ha-label component

I preferred not to do it since in some cases (like for users, quick search) ha-label is used with some fixed color...
Are you sure that "hard-coding" border inside ha-label globally is better?

P.S. Also, it is not outline, it has to be a border when used in tables.

MindFreeze · 2024-11-13T07:37:57Z

Yes. Even a fixed color can clash with the theme

MindFreeze · 2024-11-13T10:04:18Z

src/components/ha-label.ts

@@ -26,6 +26,7 @@ class HaLabel extends LitElement {
            0.15
          );
          --ha-label-background-opacity: 1;
+          border: 1px solid var(--divider-color);


why --divider-color instead of --outline-color?

Well,

In this PR a border is used instead of outline - since a parent element is clipping outline.

I got an impression that the --divider-color is mainly used for dividers/separators/etc, and the --outline-color is used mainly for outlines; also, a "plus" class for ha-label uses the --divider-color. So I thought that --divider-color fits better.

Currently both --divider-color & --outline-color have same values in a default theme - but ofc they could be different.

In the previous PR the --outline-color was used.

So, thinking about that - and probably we should:
-- either change here to --outline-color as well
-- or change to --divider-color in the 1st PR,
what do you think?

Semantically outline is the same border here, especially since we already have box-sizing: border-box, so let's just use --outline-color

OK, changed to --outline-color. Thanks for the suggestion!

MindFreeze · 2024-11-13T15:41:58Z

src/components/ha-label.ts

@@ -26,6 +26,7 @@ class HaLabel extends LitElement {
            0.15
          );
          --ha-label-background-opacity: 1;
+          border: 1px solid var(--divider-color);


Semantically outline is the same border here, especially since we already have box-sizing: border-box, so let's just use --outline-color

home-assistant · 2024-11-13T15:42:05Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

MindFreeze

We should be more consistent and change the other outlines to border but I'm not gonna nit pick that much and merge this for now

ildar170975 · 2024-11-13T16:02:11Z

change the other outlines to border

Well, let's do it! What do you think?
I intentionally wanted to use outline, honestly.

MindFreeze · 2024-11-13T16:17:29Z

change the other outlines to border

Well, let's do it! What do you think? I intentionally wanted to use outline, honestly.

Go for it if you want. But make sure to have box-sizing: border-box as well so the border doesn't change the size of the element. It may not be needed everywhere but just to be safe

ildar170975 · 2024-11-13T19:20:50Z

@MindFreeze please check #22812 if possible, thanks a lot!

ildar170975 added 7 commits November 13, 2024 06:59

Update ha-filter-labels.ts

2250a91

Update ha-automation-picker.ts

890254d

Update ha-scene-dashboard.ts

e3c7ee9

Update ha-script-picker.ts

c1268e2

Update ha-config-devices-dashboard.ts

85c446e

Update ha-config-entities.ts

5892fdf

Update ha-config-helpers.ts

8d5b17f

home-assistant bot added the cla-signed label Nov 13, 2024

MindFreeze reviewed Nov 13, 2024

View reviewed changes

ildar170975 added 7 commits November 13, 2024 11:09

Update ha-filter-labels.ts

33e938b

Update ha-automation-picker.ts

8d62722

Update ha-config-devices-dashboard.ts

9305bad

Update ha-config-helpers.ts

ea28355

Update ha-script-picker.ts

efbcf8e

Update ha-scene-dashboard.ts

5f7de53

Update ha-config-entities.ts

7d834b6

ildar170975 marked this pull request as draft November 13, 2024 08:13

ildar170975 added 2 commits November 13, 2024 11:16

Update ha-data-table-labels.ts

b1d53db

Update ha-label.ts

54dcc94

ildar170975 marked this pull request as ready for review November 13, 2024 08:20

MindFreeze reviewed Nov 13, 2024

View reviewed changes

MindFreeze requested changes Nov 13, 2024

View reviewed changes

home-assistant bot marked this pull request as draft November 13, 2024 15:42

Update ha-label.ts

1112236

ildar170975 marked this pull request as ready for review November 13, 2024 15:54

home-assistant bot requested a review from MindFreeze November 13, 2024 15:54

MindFreeze approved these changes Nov 13, 2024

View reviewed changes

MindFreeze enabled auto-merge (squash) November 13, 2024 15:59

MindFreeze merged commit 7cbebfd into home-assistant:dev Nov 13, 2024
11 checks passed

ildar170975 deleted the label_outline_2 branch November 13, 2024 16:37

ildar170975 mentioned this pull request Nov 13, 2024

Add outline to a label (3) - consistency #22812

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add outline to a label (2) #22803

Add outline to a label (2) #22803

ildar170975 commented Nov 13, 2024

coderabbitai bot commented Nov 13, 2024 •

edited

Loading

Review skipped

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

MindFreeze left a comment

ildar170975 commented Nov 13, 2024 •

edited

Loading

MindFreeze commented Nov 13, 2024

MindFreeze Nov 13, 2024

ildar170975 Nov 13, 2024

MindFreeze Nov 13, 2024

ildar170975 Nov 13, 2024

MindFreeze Nov 13, 2024

home-assistant bot commented Nov 13, 2024

MindFreeze left a comment

ildar170975 commented Nov 13, 2024

MindFreeze commented Nov 13, 2024

ildar170975 commented Nov 13, 2024

Add outline to a label (2) #22803

Add outline to a label (2) #22803

Conversation

ildar170975 commented Nov 13, 2024

Breaking change

Proposed change

Type of change

Example configuration

Additional information

Checklist

coderabbitai bot commented Nov 13, 2024 • edited Loading

Review skipped

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

MindFreeze left a comment

Choose a reason for hiding this comment

ildar170975 commented Nov 13, 2024 • edited Loading

MindFreeze commented Nov 13, 2024

MindFreeze Nov 13, 2024

Choose a reason for hiding this comment

ildar170975 Nov 13, 2024

Choose a reason for hiding this comment

MindFreeze Nov 13, 2024

Choose a reason for hiding this comment

ildar170975 Nov 13, 2024

Choose a reason for hiding this comment

MindFreeze Nov 13, 2024

Choose a reason for hiding this comment

home-assistant bot commented Nov 13, 2024

MindFreeze left a comment

Choose a reason for hiding this comment

ildar170975 commented Nov 13, 2024

MindFreeze commented Nov 13, 2024

ildar170975 commented Nov 13, 2024

coderabbitai bot commented Nov 13, 2024 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

ildar170975 commented Nov 13, 2024 •

edited

Loading