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

ES Lint rules for css-in-js declarations within Kibana #200703

Merged
merged 25 commits into from
Dec 16, 2024

Conversation

eokoneyo
Copy link
Contributor

@eokoneyo eokoneyo commented Nov 19, 2024

Summary

Closes https://github.com/elastic/kibana-team/issues/1272

This PR adds implementation for eslint rules to help facilitate the migration away from SASS files to leveraging the design tokens EUI provides for styling.

The introduced rules in this PR are as follows;

  • No CSS Color values

    Consider;

    <EuiCode style={{ color: '#dd4040' }}>Hello World!</EuiCode>

    this expression because it specifies the css color property, with a valid CSS color value, when the aforementioned rule is enabled depending on the set report level set the user would get a feedback, see screenshot below;

    Screenshot 2024-11-20 at 12 46 17

    This rule also works for variables defined elsewhere in the code and referenced as a value to the style prop, see screenshot below;

    Screenshot 2024-11-26 at 13 29 45

    feedback will also be provided when some variable that is a literal value is specified as a value for any earmarked property that should not specify literal values.

    Screenshot 2024-11-28 at 19 00 08

    feedback will be provided for referencing a member prop of some object defined elsewhere as a value to any earmarked property that we have deemed to not specify literal values

    Screenshot 2024-11-29 at 11 36 44

    Supports;

    • object values
    • object references
    • template literals
    • tagged templates

    This approach does not penalize variable declarations, only the usages of any said variable when it doesn't conform to expectation

  • Prefer CSS attributes for EUI components (optional)

    Consider;

    <EuiCode style={{ someCSSProperty: 'propertyValue' }}>Hello World!</EuiCode>

    A declaration like the one above, will be regarded as an error and can be fixed, when it's fixed it will be re-written as

    <EuiCode css={{ someCSSProperty: 'propertyValue' }}>Hello World!</EuiCode>

@eokoneyo eokoneyo added release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Nov 19, 2024
@eokoneyo eokoneyo self-assigned this Nov 19, 2024
(attr) => attr.type === 'JSXAttribute' && attr.name.name === 'css'
);

if (cssAttr) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't perform any operation when an existing css prop already exist because the eslint fixer has limited AST manipulation functionality.

@eokoneyo
Copy link
Contributor Author

/ci

@eokoneyo eokoneyo changed the title ES Lint rules for ES Lint rules for css-in-js declarations within Kibana Nov 19, 2024
@eokoneyo eokoneyo force-pushed the chore/resolve-kibana-team-1272 branch from 4386875 to c844595 Compare November 19, 2024 17:47
@eokoneyo
Copy link
Contributor Author

/ci

1 similar comment
@eokoneyo
Copy link
Contributor Author

/ci

@eokoneyo eokoneyo force-pushed the chore/resolve-kibana-team-1272 branch from aa5a2d4 to d231fe0 Compare November 20, 2024 11:48
@eokoneyo
Copy link
Contributor Author

/ci

1 similar comment
@eokoneyo
Copy link
Contributor Author

/ci

@eokoneyo eokoneyo force-pushed the chore/resolve-kibana-team-1272 branch 2 times, most recently from c364670 to 01566e0 Compare November 28, 2024 18:20
@eokoneyo eokoneyo requested a review from a team November 29, 2024 11:11
@sebelga
Copy link
Contributor

sebelga commented Dec 2, 2024

I've tested locally this PR (I know it's still in draft 😊 )
It works pretty well! Just found one thing that maybe we could improve. When we use className with a css inside, it does not complain. Do you think we could detect that too?

Screenshot 2024-12-02 at 08 43 46

@eokoneyo
Copy link
Contributor Author

eokoneyo commented Dec 2, 2024

I've tested locally this PR (I know it's still in draft 😊 ) It works pretty well! Just found one thing that maybe we could improve. When we use className with a css inside, it does not complain. Do you think we could detect that too?

Thanks for the early feedback, we can definitely account for this particular use case.

@eokoneyo eokoneyo force-pushed the chore/resolve-kibana-team-1272 branch from 3d43165 to af2c052 Compare December 2, 2024 09:19
@eokoneyo
Copy link
Contributor Author

eokoneyo commented Dec 2, 2024

I've tested locally this PR (I know it's still in draft 😊 ) It works pretty well! Just found one thing that maybe we could improve. When we use className with a css inside, it does not complain. Do you think we could detect that too?

Thanks for the early feedback, we can definitely account for this particular use case.

accounted for in af2c052 (#200703)

@eokoneyo eokoneyo force-pushed the chore/resolve-kibana-team-1272 branch from 774279d to 3ade9d2 Compare December 2, 2024 09:48
@eokoneyo
Copy link
Contributor Author

eokoneyo commented Dec 2, 2024

/ci

@eokoneyo eokoneyo force-pushed the chore/resolve-kibana-team-1272 branch from d0956af to 17f22f9 Compare December 2, 2024 12:13
@eokoneyo
Copy link
Contributor Author

eokoneyo commented Dec 2, 2024

/ci

3 similar comments
@eokoneyo
Copy link
Contributor Author

eokoneyo commented Dec 4, 2024

/ci

@eokoneyo
Copy link
Contributor Author

eokoneyo commented Dec 5, 2024

/ci

@eokoneyo
Copy link
Contributor Author

eokoneyo commented Dec 5, 2024

/ci

@eokoneyo eokoneyo force-pushed the chore/resolve-kibana-team-1272 branch from f2e2f0c to 21edadd Compare December 10, 2024 14:32
@eokoneyo
Copy link
Contributor Author

/ci

Copy link
Contributor

@clintandrewhall clintandrewhall left a comment

Choose a reason for hiding this comment

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

Excellent work here. I'm sure we'll make improvements as people start to use it, but getting it out and covering the obvious cases is awesome.

/**
* @description List of css properties that can that can apply color to html box element and text node
*/
const propertiesSupportingCssColor = ['color', 'background', 'backgroundColor', 'borderColor'];
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: technically, also border and border-[side]-color

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switched this implementation, so now only specifying border, background and border should suffice, let me know if I missed something

@eokoneyo eokoneyo force-pushed the chore/resolve-kibana-team-1272 branch from f715fcc to dfea8c2 Compare December 12, 2024 12:40
@eokoneyo
Copy link
Contributor Author

/ci

@eokoneyo eokoneyo marked this pull request as ready for review December 12, 2024 15:25
@eokoneyo eokoneyo requested a review from a team as a code owner December 12, 2024 15:25
@elasticmachine
Copy link
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

@eokoneyo eokoneyo requested a review from a team December 16, 2024 09:16
@eokoneyo
Copy link
Contributor Author

@elasticmachine merge upstream

@eokoneyo
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #66 / Entity Manager _count API is resilient to no valid sources

Metrics [docs]

✅ unchanged

History

cc @eokoneyo

@eokoneyo eokoneyo merged commit 7370cc7 into elastic:main Dec 16, 2024
8 checks passed
@eokoneyo eokoneyo deleted the chore/resolve-kibana-team-1272 branch December 16, 2024 15:56
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12356395531

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 16, 2024
## Summary

Closes elastic/kibana-team#1272

This PR adds implementation for eslint rules to help facilitate the
migration away from SASS files to leveraging the design tokens EUI
provides for styling.

The introduced rules  in this PR are as follows;

- #### No CSS Color values
  Consider;

  ```tsx
  <EuiCode style={{ color: '#dd4040' }}>Hello World!</EuiCode>
  ```

this expression because it specifies the css color property, with a
valid [CSS color
value](https://developer.mozilla.org/en-US/docs/Web/CSS/color_value),
when the aforementioned rule is enabled depending on the set report
level set the user would get a feedback, see screenshot below;

<img width="735" alt="Screenshot 2024-11-20 at 12 46 17"
src="https://github.com/user-attachments/assets/d2f608dc-782c-4d83-88e6-92dfdd8f6101">

This rule also works for variables defined elsewhere in the code and
referenced as a value to the style prop, see screenshot below;

<img width="1658" alt="Screenshot 2024-11-26 at 13 29 45"
src="https://github.com/user-attachments/assets/f8aadf6b-318b-4c6a-b7c9-bb44fb867b58">

feedback will also be provided when some variable that is a literal
value is specified as a value for any earmarked property that should not
specify literal values.

<img width="1730" alt="Screenshot 2024-11-28 at 19 00 08"
src="https://github.com/user-attachments/assets/bc3a8674-9469-4c7a-b0c9-7a2bfa7f08dc">

feedback will be provided for referencing a member prop of some object
defined elsewhere as a value to any earmarked property that we have
deemed to not specify literal values

<img width="1676" alt="Screenshot 2024-11-29 at 11 36 44"
src="https://github.com/user-attachments/assets/c4537fbf-b2d8-46bb-ad5f-8582e8c9a932">

	Supports;
	- object values
	- object references
	- template literals
	- tagged templates

This approach does not penalize variable declarations, only the usages
of any said variable when it doesn't conform to expectation

- #### Prefer CSS attributes for EUI components (optional)
  Consider;

  ```tsx
<EuiCode style={{ someCSSProperty: 'propertyValue' }}>Hello
World!</EuiCode>
  ```
A declaration like the one above, will be regarded as an error and can
be fixed, when it's fixed it will be re-written as

  ```tsx
<EuiCode css={{ someCSSProperty: 'propertyValue' }}>Hello
World!</EuiCode>
  ```

<!--

### Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] The PR description includes the appropriate Release Notes section,
and the correct `release_node:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

### Identify risks

Does this PR introduce any risks? For example, consider risks like hard
to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified
risk. Invite stakeholders and evaluate how to proceed before merging.

- [ ] [See some risk
examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
- [ ] ...

-->

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
(cherry picked from commit 7370cc7)
@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.x An unhandled error occurred. Please see the logs for details

Manual backport

To create the backport manually run:

node scripts/backport --pr 200703

Questions ?

Please refer to the Backport tool documentation

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 200703 locally

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 17, 2024
@eokoneyo
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

stephmilovic pushed a commit that referenced this pull request Dec 18, 2024
#204620)

# Backport

This will backport the following commits from `main` to `8.x`:
- [ES Lint rules for css-in-js declarations within Kibana
(#200703)](#200703)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Eyo O.
Eyo","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-16T15:56:52Z","message":"ES
Lint rules for css-in-js declarations within Kibana (#200703)\n\n##
Summary\r\n\r\nCloses
https://github.com/elastic/kibana-team/issues/1272\r\n\r\nThis PR adds
implementation for eslint rules to help facilitate the\r\nmigration away
from SASS files to leveraging the design tokens EUI\r\nprovides for
styling.\r\n\r\nThe introduced rules in this PR are as follows;\r\n\r\n-
#### No CSS Color values\r\n Consider; \r\n\r\n ```tsx\r\n <EuiCode
style={{ color: '#dd4040' }}>Hello World!</EuiCode>\r\n ```\r\n\r\nthis
expression because it specifies the css color property, with a\r\nvalid
[CSS
color\r\nvalue](https://developer.mozilla.org/en-US/docs/Web/CSS/color_value),\r\nwhen
the aforementioned rule is enabled depending on the set report\r\nlevel
set the user would get a feedback, see screenshot below;\r\n\r\n\r\n<img
width=\"735\" alt=\"Screenshot 2024-11-20 at 12 46
17\"\r\nsrc=\"https://github.com/user-attachments/assets/d2f608dc-782c-4d83-88e6-92dfdd8f6101\">\r\n\r\nThis
rule also works for variables defined elsewhere in the code
and\r\nreferenced as a value to the style prop, see screenshot
below;\r\n\r\n<img width=\"1658\" alt=\"Screenshot 2024-11-26 at 13 29
45\"\r\nsrc=\"https://github.com/user-attachments/assets/f8aadf6b-318b-4c6a-b7c9-bb44fb867b58\">\r\n\r\nfeedback
will also be provided when some variable that is a literal\r\nvalue is
specified as a value for any earmarked property that should
not\r\nspecify literal values.\r\n\r\n<img width=\"1730\"
alt=\"Screenshot 2024-11-28 at 19 00
08\"\r\nsrc=\"https://github.com/user-attachments/assets/bc3a8674-9469-4c7a-b0c9-7a2bfa7f08dc\">\r\n\r\nfeedback
will be provided for referencing a member prop of some object\r\ndefined
elsewhere as a value to any earmarked property that we have\r\ndeemed to
not specify literal values\r\n\r\n<img width=\"1676\" alt=\"Screenshot
2024-11-29 at 11 36
44\"\r\nsrc=\"https://github.com/user-attachments/assets/c4537fbf-b2d8-46bb-ad5f-8582e8c9a932\">\r\n\r\n\tSupports;\r\n\t-
object values\r\n\t- object references\r\n\t- template literals\r\n\t-
tagged templates \r\n\r\nThis approach does not penalize variable
declarations, only the usages\r\nof any said variable when it doesn't
conform to expectation\r\n\r\n- #### Prefer CSS attributes for EUI
components (optional)\r\n Consider; \r\n\r\n ```tsx\r\n<EuiCode style={{
someCSSProperty: 'propertyValue' }}>Hello\r\nWorld!</EuiCode>\r\n
```\r\nA declaration like the one above, will be regarded as an error
and can\r\nbe fixed, when it's fixed it will be re-written as\r\n \r\n
```tsx\r\n<EuiCode css={{ someCSSProperty: 'propertyValue'
}}>Hello\r\nWorld!</EuiCode>\r\n ```\r\n\r\n\t\r\n<!--\r\n\r\n###
Checklist\r\n\r\nCheck the PR satisfies following conditions.
\r\n\r\nReviewers should verify this PR satisfies this list as
well.\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] If a plugin
configuration key changed, check if it needs to be\r\nallowlisted in the
cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This was checked for breaking HTTP API changes, and any
breaking\r\nchanges have been approved by the breaking-change committee.
The\r\n`release_note:breaking` label should be applied in these
situations.\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] The PR description includes
the appropriate Release Notes section,\r\nand the correct
`release_node:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n###
Identify risks\r\n\r\nDoes this PR introduce any risks? For example,
consider risks like hard\r\nto test bugs, performance regression,
potential of data loss.\r\n\r\nDescribe the risk, its severity, and
mitigation for each identified\r\nrisk. Invite stakeholders and evaluate
how to proceed before merging.\r\n\r\n- [ ] [See some
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\r\n-
[ ] ...\r\n\r\n\r\n-->\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"7370cc712ee2bf23f4cf53eef2e74942340f761c","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport
missing","v9.0.0","Team:SharedUX","backport:prev-minor"],"number":200703,"url":"https://github.com/elastic/kibana/pull/200703","mergeCommit":{"message":"ES
Lint rules for css-in-js declarations within Kibana (#200703)\n\n##
Summary\r\n\r\nCloses
https://github.com/elastic/kibana-team/issues/1272\r\n\r\nThis PR adds
implementation for eslint rules to help facilitate the\r\nmigration away
from SASS files to leveraging the design tokens EUI\r\nprovides for
styling.\r\n\r\nThe introduced rules in this PR are as follows;\r\n\r\n-
#### No CSS Color values\r\n Consider; \r\n\r\n ```tsx\r\n <EuiCode
style={{ color: '#dd4040' }}>Hello World!</EuiCode>\r\n ```\r\n\r\nthis
expression because it specifies the css color property, with a\r\nvalid
[CSS
color\r\nvalue](https://developer.mozilla.org/en-US/docs/Web/CSS/color_value),\r\nwhen
the aforementioned rule is enabled depending on the set report\r\nlevel
set the user would get a feedback, see screenshot below;\r\n\r\n\r\n<img
width=\"735\" alt=\"Screenshot 2024-11-20 at 12 46
17\"\r\nsrc=\"https://github.com/user-attachments/assets/d2f608dc-782c-4d83-88e6-92dfdd8f6101\">\r\n\r\nThis
rule also works for variables defined elsewhere in the code
and\r\nreferenced as a value to the style prop, see screenshot
below;\r\n\r\n<img width=\"1658\" alt=\"Screenshot 2024-11-26 at 13 29
45\"\r\nsrc=\"https://github.com/user-attachments/assets/f8aadf6b-318b-4c6a-b7c9-bb44fb867b58\">\r\n\r\nfeedback
will also be provided when some variable that is a literal\r\nvalue is
specified as a value for any earmarked property that should
not\r\nspecify literal values.\r\n\r\n<img width=\"1730\"
alt=\"Screenshot 2024-11-28 at 19 00
08\"\r\nsrc=\"https://github.com/user-attachments/assets/bc3a8674-9469-4c7a-b0c9-7a2bfa7f08dc\">\r\n\r\nfeedback
will be provided for referencing a member prop of some object\r\ndefined
elsewhere as a value to any earmarked property that we have\r\ndeemed to
not specify literal values\r\n\r\n<img width=\"1676\" alt=\"Screenshot
2024-11-29 at 11 36
44\"\r\nsrc=\"https://github.com/user-attachments/assets/c4537fbf-b2d8-46bb-ad5f-8582e8c9a932\">\r\n\r\n\tSupports;\r\n\t-
object values\r\n\t- object references\r\n\t- template literals\r\n\t-
tagged templates \r\n\r\nThis approach does not penalize variable
declarations, only the usages\r\nof any said variable when it doesn't
conform to expectation\r\n\r\n- #### Prefer CSS attributes for EUI
components (optional)\r\n Consider; \r\n\r\n ```tsx\r\n<EuiCode style={{
someCSSProperty: 'propertyValue' }}>Hello\r\nWorld!</EuiCode>\r\n
```\r\nA declaration like the one above, will be regarded as an error
and can\r\nbe fixed, when it's fixed it will be re-written as\r\n \r\n
```tsx\r\n<EuiCode css={{ someCSSProperty: 'propertyValue'
}}>Hello\r\nWorld!</EuiCode>\r\n ```\r\n\r\n\t\r\n<!--\r\n\r\n###
Checklist\r\n\r\nCheck the PR satisfies following conditions.
\r\n\r\nReviewers should verify this PR satisfies this list as
well.\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] If a plugin
configuration key changed, check if it needs to be\r\nallowlisted in the
cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This was checked for breaking HTTP API changes, and any
breaking\r\nchanges have been approved by the breaking-change committee.
The\r\n`release_note:breaking` label should be applied in these
situations.\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] The PR description includes
the appropriate Release Notes section,\r\nand the correct
`release_node:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n###
Identify risks\r\n\r\nDoes this PR introduce any risks? For example,
consider risks like hard\r\nto test bugs, performance regression,
potential of data loss.\r\n\r\nDescribe the risk, its severity, and
mitigation for each identified\r\nrisk. Invite stakeholders and evaluate
how to proceed before merging.\r\n\r\n- [ ] [See some
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\r\n-
[ ] ...\r\n\r\n\r\n-->\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"7370cc712ee2bf23f4cf53eef2e74942340f761c"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/200703","number":200703,"mergeCommit":{"message":"ES
Lint rules for css-in-js declarations within Kibana (#200703)\n\n##
Summary\r\n\r\nCloses
https://github.com/elastic/kibana-team/issues/1272\r\n\r\nThis PR adds
implementation for eslint rules to help facilitate the\r\nmigration away
from SASS files to leveraging the design tokens EUI\r\nprovides for
styling.\r\n\r\nThe introduced rules in this PR are as follows;\r\n\r\n-
#### No CSS Color values\r\n Consider; \r\n\r\n ```tsx\r\n <EuiCode
style={{ color: '#dd4040' }}>Hello World!</EuiCode>\r\n ```\r\n\r\nthis
expression because it specifies the css color property, with a\r\nvalid
[CSS
color\r\nvalue](https://developer.mozilla.org/en-US/docs/Web/CSS/color_value),\r\nwhen
the aforementioned rule is enabled depending on the set report\r\nlevel
set the user would get a feedback, see screenshot below;\r\n\r\n\r\n<img
width=\"735\" alt=\"Screenshot 2024-11-20 at 12 46
17\"\r\nsrc=\"https://github.com/user-attachments/assets/d2f608dc-782c-4d83-88e6-92dfdd8f6101\">\r\n\r\nThis
rule also works for variables defined elsewhere in the code
and\r\nreferenced as a value to the style prop, see screenshot
below;\r\n\r\n<img width=\"1658\" alt=\"Screenshot 2024-11-26 at 13 29
45\"\r\nsrc=\"https://github.com/user-attachments/assets/f8aadf6b-318b-4c6a-b7c9-bb44fb867b58\">\r\n\r\nfeedback
will also be provided when some variable that is a literal\r\nvalue is
specified as a value for any earmarked property that should
not\r\nspecify literal values.\r\n\r\n<img width=\"1730\"
alt=\"Screenshot 2024-11-28 at 19 00
08\"\r\nsrc=\"https://github.com/user-attachments/assets/bc3a8674-9469-4c7a-b0c9-7a2bfa7f08dc\">\r\n\r\nfeedback
will be provided for referencing a member prop of some object\r\ndefined
elsewhere as a value to any earmarked property that we have\r\ndeemed to
not specify literal values\r\n\r\n<img width=\"1676\" alt=\"Screenshot
2024-11-29 at 11 36
44\"\r\nsrc=\"https://github.com/user-attachments/assets/c4537fbf-b2d8-46bb-ad5f-8582e8c9a932\">\r\n\r\n\tSupports;\r\n\t-
object values\r\n\t- object references\r\n\t- template literals\r\n\t-
tagged templates \r\n\r\nThis approach does not penalize variable
declarations, only the usages\r\nof any said variable when it doesn't
conform to expectation\r\n\r\n- #### Prefer CSS attributes for EUI
components (optional)\r\n Consider; \r\n\r\n ```tsx\r\n<EuiCode style={{
someCSSProperty: 'propertyValue' }}>Hello\r\nWorld!</EuiCode>\r\n
```\r\nA declaration like the one above, will be regarded as an error
and can\r\nbe fixed, when it's fixed it will be re-written as\r\n \r\n
```tsx\r\n<EuiCode css={{ someCSSProperty: 'propertyValue'
}}>Hello\r\nWorld!</EuiCode>\r\n ```\r\n\r\n\t\r\n<!--\r\n\r\n###
Checklist\r\n\r\nCheck the PR satisfies following conditions.
\r\n\r\nReviewers should verify this PR satisfies this list as
well.\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] If a plugin
configuration key changed, check if it needs to be\r\nallowlisted in the
cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This was checked for breaking HTTP API changes, and any
breaking\r\nchanges have been approved by the breaking-change committee.
The\r\n`release_note:breaking` label should be applied in these
situations.\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] The PR description includes
the appropriate Release Notes section,\r\nand the correct
`release_node:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n###
Identify risks\r\n\r\nDoes this PR introduce any risks? For example,
consider risks like hard\r\nto test bugs, performance regression,
potential of data loss.\r\n\r\nDescribe the risk, its severity, and
mitigation for each identified\r\nrisk. Invite stakeholders and evaluate
how to proceed before merging.\r\n\r\n- [ ] [See some
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\r\n-
[ ] ...\r\n\r\n\r\n-->\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"7370cc712ee2bf23f4cf53eef2e74942340f761c"}}]}]
BACKPORT-->
@kibanamachine kibanamachine added v8.18.0 and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Dec 18, 2024
JoseLuisGJ pushed a commit to JoseLuisGJ/kibana that referenced this pull request Dec 19, 2024
## Summary

Closes elastic/kibana-team#1272

This PR adds implementation for eslint rules to help facilitate the
migration away from SASS files to leveraging the design tokens EUI
provides for styling.

The introduced rules  in this PR are as follows;

- #### No CSS Color values
  Consider; 

  ```tsx
  <EuiCode style={{ color: '#dd4040' }}>Hello World!</EuiCode>
  ```

this expression because it specifies the css color property, with a
valid [CSS color
value](https://developer.mozilla.org/en-US/docs/Web/CSS/color_value),
when the aforementioned rule is enabled depending on the set report
level set the user would get a feedback, see screenshot below;


<img width="735" alt="Screenshot 2024-11-20 at 12 46 17"
src="https://github.com/user-attachments/assets/d2f608dc-782c-4d83-88e6-92dfdd8f6101">

This rule also works for variables defined elsewhere in the code and
referenced as a value to the style prop, see screenshot below;

<img width="1658" alt="Screenshot 2024-11-26 at 13 29 45"
src="https://github.com/user-attachments/assets/f8aadf6b-318b-4c6a-b7c9-bb44fb867b58">

feedback will also be provided when some variable that is a literal
value is specified as a value for any earmarked property that should not
specify literal values.

<img width="1730" alt="Screenshot 2024-11-28 at 19 00 08"
src="https://github.com/user-attachments/assets/bc3a8674-9469-4c7a-b0c9-7a2bfa7f08dc">

feedback will be provided for referencing a member prop of some object
defined elsewhere as a value to any earmarked property that we have
deemed to not specify literal values

<img width="1676" alt="Screenshot 2024-11-29 at 11 36 44"
src="https://github.com/user-attachments/assets/c4537fbf-b2d8-46bb-ad5f-8582e8c9a932">

	Supports;
	- object values
	- object references
	- template literals
	- tagged templates 

This approach does not penalize variable declarations, only the usages
of any said variable when it doesn't conform to expectation

- #### Prefer CSS attributes for EUI components (optional)
  Consider; 

  ```tsx
<EuiCode style={{ someCSSProperty: 'propertyValue' }}>Hello
World!</EuiCode>
  ```
A declaration like the one above, will be regarded as an error and can
be fixed, when it's fixed it will be re-written as
  
  ```tsx
<EuiCode css={{ someCSSProperty: 'propertyValue' }}>Hello
World!</EuiCode>
  ```

	
<!--

### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] The PR description includes the appropriate Release Notes section,
and the correct `release_node:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

### Identify risks

Does this PR introduce any risks? For example, consider risks like hard
to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified
risk. Invite stakeholders and evaluate how to proceed before merging.

- [ ] [See some risk
examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
- [ ] ...


-->

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Jan 13, 2025
## Summary

Closes elastic/kibana-team#1272

This PR adds implementation for eslint rules to help facilitate the
migration away from SASS files to leveraging the design tokens EUI
provides for styling.

The introduced rules  in this PR are as follows;

- #### No CSS Color values
  Consider; 

  ```tsx
  <EuiCode style={{ color: '#dd4040' }}>Hello World!</EuiCode>
  ```

this expression because it specifies the css color property, with a
valid [CSS color
value](https://developer.mozilla.org/en-US/docs/Web/CSS/color_value),
when the aforementioned rule is enabled depending on the set report
level set the user would get a feedback, see screenshot below;


<img width="735" alt="Screenshot 2024-11-20 at 12 46 17"
src="https://github.com/user-attachments/assets/d2f608dc-782c-4d83-88e6-92dfdd8f6101">

This rule also works for variables defined elsewhere in the code and
referenced as a value to the style prop, see screenshot below;

<img width="1658" alt="Screenshot 2024-11-26 at 13 29 45"
src="https://github.com/user-attachments/assets/f8aadf6b-318b-4c6a-b7c9-bb44fb867b58">

feedback will also be provided when some variable that is a literal
value is specified as a value for any earmarked property that should not
specify literal values.

<img width="1730" alt="Screenshot 2024-11-28 at 19 00 08"
src="https://github.com/user-attachments/assets/bc3a8674-9469-4c7a-b0c9-7a2bfa7f08dc">

feedback will be provided for referencing a member prop of some object
defined elsewhere as a value to any earmarked property that we have
deemed to not specify literal values

<img width="1676" alt="Screenshot 2024-11-29 at 11 36 44"
src="https://github.com/user-attachments/assets/c4537fbf-b2d8-46bb-ad5f-8582e8c9a932">

	Supports;
	- object values
	- object references
	- template literals
	- tagged templates 

This approach does not penalize variable declarations, only the usages
of any said variable when it doesn't conform to expectation

- #### Prefer CSS attributes for EUI components (optional)
  Consider; 

  ```tsx
<EuiCode style={{ someCSSProperty: 'propertyValue' }}>Hello
World!</EuiCode>
  ```
A declaration like the one above, will be regarded as an error and can
be fixed, when it's fixed it will be re-written as
  
  ```tsx
<EuiCode css={{ someCSSProperty: 'propertyValue' }}>Hello
World!</EuiCode>
  ```

	
<!--

### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] The PR description includes the appropriate Release Notes section,
and the correct `release_node:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

### Identify risks

Does this PR introduce any risks? For example, consider risks like hard
to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified
risk. Invite stakeholders and evaluate how to proceed before merging.

- [ ] [See some risk
examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
- [ ] ...


-->

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants