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

Help developers in VS Code with guidance for failures and warnings #7506

Closed
4 of 5 tasks
Tracked by #7503
alex-page opened this issue Oct 25, 2022 · 6 comments · Fixed by #7696
Closed
4 of 5 tasks
Tracked by #7503

Help developers in VS Code with guidance for failures and warnings #7506

alex-page opened this issue Oct 25, 2022 · 6 comments · Fixed by #7696
Assignees

Comments

@alex-page
Copy link
Member

alex-page commented Oct 25, 2022

#7506 (comment)

VS Code Example Website Example
Image Image Image
@sophschneider
Copy link
Contributor

Does this mean:

  • the linter error output?
  • the underline popover thing in VSCode?
  • the quick fix option in VSCode?
  • the --fix option on stylelint
  • the coverage site?
  • links to documentation?

@sophschneider
Copy link
Contributor

https://stylelint.io/developer-guide/formatters/ can help us write meaningful error message

@alex-page alex-page changed the title Add world class guidance for failures and warnings that help developers resolve problems quickly Help developers in VSCode with guidance for failures and warnings Nov 7, 2022
@alex-page
Copy link
Member Author

alex-page commented Nov 7, 2022

@sophschneider Added some clarity around the steps

@sam-b-rose sam-b-rose changed the title Help developers in VSCode with guidance for failures and warnings Help developers in VS Code with guidance for failures and warnings Nov 7, 2022
@chloerice
Copy link
Member

chloerice commented Nov 10, 2022

I've got a PR up in the Stylelint repo extending the recently added support for message arguments for

  • declaration-property-value-disallowed-list
  • declaration-property-unit-disallowed-list
  • function-disallowed-list
  • at-rule-disallowed-list
  • property-disallowed-list

This will allow me to add helpful custom messages alongside autofixes. Will pair with @aaronccasanova to add a patch to our repo in the interim of their next release.

@sam-b-rose
Copy link
Member

sam-b-rose commented Dec 12, 2022

For the coverage rules where we are leveraging Stylelint built-in rules, is it possible to pass along the URL link to the Stylelint rule docs. Could we also leverage the --fix option for those rules? 🤔

This applies mostly to coverage rules likecolor-no-hex and less so to more custom rules like custom-properties-allowed-list.

@aaronccasanova
Copy link
Member

Could we also leverage the --fix option for those rules?

Yes, support for autofixing built-in and custom rules was added here: stylelint/stylelint#6466

chloerice added a commit that referenced this issue Dec 13, 2022
### WHY are these changes introduced?

Closes #7506 

#### To do

- [x] Bring in changes from
https://github.com/Shopify/polaris/pull/7814/files
- [ ] ~Finish adding examples to coverage/README.md~ (handling in
separate [PR](#7878))
- [x] Update screenshots to reflect current approach
- [x] Update/add comments to notable changes/fixes/open questions

### WHAT is this pull request doing?

This pull request adds support for configuring
`stylelint-polaris/coverage` rules with custom messages and metadata, so
that error and warning messages tell admin builders how to resolve the
problem and VS Code diagnostics link to the `@shopify/stylelint-polaris`
documentation.

For example: 

<img width="610" alt="Screenshot 2022-12-09 at 1 41 40 PM"
src="https://user-images.githubusercontent.com/18447883/206791342-cc12ccab-9d36-44db-a619-d7c59dd2b265.png">

This PR also includes a bit of polish:

- Added typing and descriptions to `polaris/coverage` plugin so that
primary options are clear in the config and in the plugin's code
- Updated plugin names to be singular instead of plural 

```diff
- custom-properties-allowed-list
+ custom-property-allowed-list

- media-queries-allowed-list
+ media-query-allowed-list
```
     
     

<!--
  Summary of the changes committed.

Before / after screenshots are appreciated for UI changes. Make sure to
include alt text that describes the screenshot.

If you include an animated gif showing your change, wrapping it in a
details tag is recommended. Gifs usually autoplay, which can cause
accessibility issues for people reviewing your PR:

    <details>
      <summary>Summary of your gif(s)</summary>
      <img src="..." alt="Description of what the gif shows">
    </details>
-->

<!-- ℹ️ Delete the following for small / trivial changes -->

### How to 🎩

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

<!--
  Give as much information as needed to experiment with the component
  in the playground.
-->

<details>
<summary>Copy-paste this code in
<code>playground/Playground.tsx</code>:</summary>

```jsx
import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}
```

</details>

### 🎩 checklist

- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
juzser added a commit to juzser/polaris that referenced this issue Jul 27, 2023
Closes Shopify#7506

- [x] Bring in changes from
https://github.com/Shopify/polaris/pull/7814/files
- [ ] ~Finish adding examples to coverage/README.md~ (handling in
separate [PR](Shopify#7878))
- [x] Update screenshots to reflect current approach
- [x] Update/add comments to notable changes/fixes/open questions

This pull request adds support for configuring
`stylelint-polaris/coverage` rules with custom messages and metadata, so
that error and warning messages tell admin builders how to resolve the
problem and VS Code diagnostics link to the `@shopify/stylelint-polaris`
documentation.

For example:

<img width="610" alt="Screenshot 2022-12-09 at 1 41 40 PM"
src="https://user-images.githubusercontent.com/18447883/206791342-cc12ccab-9d36-44db-a619-d7c59dd2b265.png">

This PR also includes a bit of polish:

- Added typing and descriptions to `polaris/coverage` plugin so that
primary options are clear in the config and in the plugin's code
- Updated plugin names to be singular instead of plural

```diff
- custom-properties-allowed-list
+ custom-property-allowed-list

- media-queries-allowed-list
+ media-query-allowed-list
```

<!--
  Summary of the changes committed.

Before / after screenshots are appreciated for UI changes. Make sure to
include alt text that describes the screenshot.

If you include an animated gif showing your change, wrapping it in a
details tag is recommended. Gifs usually autoplay, which can cause
accessibility issues for people reviewing your PR:

    <details>
      <summary>Summary of your gif(s)</summary>
      <img src="..." alt="Description of what the gif shows">
    </details>
-->

<!-- ℹ️ Delete the following for small / trivial changes -->

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

<!--
  Give as much information as needed to experiment with the component
  in the playground.
-->

<details>
<summary>Copy-paste this code in
<code>playground/Playground.tsx</code>:</summary>

```jsx
import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}
```

</details>

- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants