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

Remove all the linting warnings #1108

Open
bfabio opened this issue Nov 30, 2023 · 4 comments
Open

Remove all the linting warnings #1108

bfabio opened this issue Nov 30, 2023 · 4 comments

Comments

@bfabio
Copy link
Member

bfabio commented Nov 30, 2023

There are a few linting warning remaining that pollute our npm run lint output and are most likely symptoms of actual bugs , we should fix those - preferably not by disabling them:

npm run lint

> [email protected] lint
> eslint --ignore-path .gitignore .


/home/fabio/dev/designers.italia.it/src/components/checkbox/checkbox.js
  18:5  warning  Visible, non-interactive elements with click handlers must have at least one keyboard listener                                                                                                     jsx-a11y/click-events-have-key-events
  18:5  warning  Avoid non-native interactive elements. If using native HTML is not possible, add an appropriate role and support for tabbing, mouse, keyboard, and touch inputs to an interactive content element  jsx-a11y/no-static-element-interactions

/home/fabio/dev/designers.italia.it/src/components/content-collapse/contentCollapse.js
  49:7  warning  Anchor used as a button. Anchors are primarily expected to navigate. Use the button element instead. Learn more: https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/HEAD/docs/rules/anchor-is-valid.md  jsx-a11y/anchor-is-valid

/home/fabio/dev/designers.italia.it/src/components/feedback/components/form-no/FormNo.js
   85:15  warning  A form label must be associated with a control  jsx-a11y/label-has-associated-control
   94:15  warning  A form label must be associated with a control  jsx-a11y/label-has-associated-control
  103:15  warning  A form label must be associated with a control  jsx-a11y/label-has-associated-control
  112:15  warning  A form label must be associated with a control  jsx-a11y/label-has-associated-control
  121:15  warning  A form label must be associated with a control  jsx-a11y/label-has-associated-control
  130:15  warning  A form label must be associated with a control  jsx-a11y/label-has-associated-control
  139:15  warning  A form label must be associated with a control  jsx-a11y/label-has-associated-control
  148:15  warning  A form label must be associated with a control  jsx-a11y/label-has-associated-control
  157:15  warning  A form label must be associated with a control  jsx-a11y/label-has-associated-control
  166:15  warning  A form label must be associated with a control  jsx-a11y/label-has-associated-control
  175:15  warning  A form label must be associated with a control  jsx-a11y/label-has-associated-control
  197:15  warning  A form label must be associated with a control  jsx-a11y/label-has-associated-control
  206:15  warning  A form label must be associated with a control  jsx-a11y/label-has-associated-control
  215:15  warning  A form label must be associated with a control  jsx-a11y/label-has-associated-control
  224:15  warning  A form label must be associated with a control  jsx-a11y/label-has-associated-control
  233:15  warning  A form label must be associated with a control  jsx-a11y/label-has-associated-control
  242:15  warning  A form label must be associated with a control  jsx-a11y/label-has-associated-control
  251:15  warning  A form label must be associated with a control  jsx-a11y/label-has-associated-control

/home/fabio/dev/designers.italia.it/src/components/feedback/feedback.js
   63:9   warning  Unexpected console statement                    no-console
  132:19  warning  A form label must be associated with a control  jsx-a11y/label-has-associated-control
  144:19  warning  A form label must be associated with a control  jsx-a11y/label-has-associated-control

/home/fabio/dev/designers.italia.it/src/components/footer-main/footer-main.js
  72:45  warning  'index' is already declared in the upper scope on line 66 column 33  no-shadow

/home/fabio/dev/designers.italia.it/src/components/list-item/list-item.js
   95:26  warning  'icons' is already declared in the upper scope on line 92 column 7                                                                                                                                                                                                                                                                                                 no-shadow
  122:26  warning  The href attribute requires a valid value to be accessible. Provide a valid, navigable address as the href value. If you cannot provide a valid href, but still need the element to resemble a link, use a button and change it with appropriate styles. Learn more: https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/HEAD/docs/rules/anchor-is-valid.md  jsx-a11y/anchor-is-valid

/home/fabio/dev/designers.italia.it/src/components/media-player/media-player.js
  106:11  warning  Media elements such as <audio> and <video> must have a <track> for captions  jsx-a11y/media-has-caption

/home/fabio/dev/designers.italia.it/src/components/nav-sidebar/nav-sidebar.js
  200:15  warning  The href attribute requires a valid value to be accessible. Provide a valid, navigable address as the href value. If you cannot provide a valid href, but still need the element to resemble a link, use a button and change it with appropriate styles. Learn more: https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/HEAD/docs/rules/anchor-is-valid.md  jsx-a11y/anchor-is-valid

/home/fabio/dev/designers.italia.it/src/components/search-main/search-main.js
  113:51  warning  'index' is already declared in the upper scope on line 31 column 25  no-shadow
  137:23  warning  A form label must be associated with a control                       jsx-a11y/label-has-associated-control

/home/fabio/dev/designers.italia.it/src/components/section-editorial/section-editorial.js
  107:25  warning  The href attribute requires a valid value to be accessible. Provide a valid, navigable address as the href value. If you cannot provide a valid href, but still need the element to resemble a link, use a button and change it with appropriate styles. Learn more: https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/HEAD/docs/rules/anchor-is-valid.md  jsx-a11y/anchor-is-valid
  112:25  warning  The href attribute requires a valid value to be accessible. Provide a valid, navigable address as the href value. If you cannot provide a valid href, but still need the element to resemble a link, use a button and change it with appropriate styles. Learn more: https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/HEAD/docs/rules/anchor-is-valid.md  jsx-a11y/anchor-is-valid

/home/fabio/dev/designers.italia.it/src/components/table/table.js
  29:36  warning  'index' is already declared in the upper scope on line 27 column 35  no-shadow
  37:53  warning  'index' is already declared in the upper scope on line 29 column 36  no-shadow

✖ 35 problems (0 errors, 35 warnings)
@Fupete Fupete linked a pull request Dec 1, 2023 that will close this issue
@Fupete
Copy link
Collaborator

Fupete commented Dec 1, 2023

Update: this PR #1112 fixed half of the warnings as you can see:

updat

@bfabio I leave the correction of :

  • checkbox.js
  • formNo.js (should be renamed form-no.js)
  • feedback.js
  • search-main.js

Finally I have a couple of doubts about media-player.js, perhaps we should consider disabling this lint:

  • some of the old archive media we retrieved are without subtitles;
  • however, for the new ones, we load subtitles via the dedicated API method of video.js (line 53) within useEffect as you can see here:
if (subtitles)
      video.player.addRemoteTextTrack({
        kind: "subtitles",
        label: "Italiano",
        srclang: "it",
        default: true,
        src: subtitles,
      });

Update: we need to investigate further the warning A form label must be associated with a control jsx-a11y/label-has-associated-control beause each labels seems to have its correct htmlFor property...

@Fupete Fupete moved this to 🛠 Doing in Designers Italia - Website Dec 1, 2023
@Fupete Fupete added bug dependencies Pull requests that update a dependency file a11y labels Dec 1, 2023
@Fupete

This comment was marked as outdated.

@Fupete Fupete added enhancement help wanted and removed bug dependencies Pull requests that update a dependency file labels Dec 1, 2023
@Fupete Fupete removed a link to a pull request Dec 18, 2023
@Fupete Fupete moved this from 🛠 Doing to ✏ Todo in Designers Italia - Website Dec 18, 2023
@Fupete
Copy link
Collaborator

Fupete commented Dec 18, 2023

@bfabio I just updated the comment above after merging the PR that fix half of the warnings.

@bfabio
Copy link
Member Author

bfabio commented Jun 13, 2024

Finally I have a couple of doubts about media-player.js, perhaps we should consider disabling this lint:

  • some of the old archive media we retrieved are without subtitles;

  • however, for the new ones, we load subtitles via the dedicated API method of video.js (line 53) within useEffect as you can see here:

if (subtitles)
      video.player.addRemoteTextTrack({
        kind: "subtitles",
        label: "Italiano",
        srclang: "it",
        default: true,
        src: subtitles,
      });

Thinking about it, videos not having the subtitles should be handled as an error IMO. So, the possible steps:

  1. Create an issue to subtitle all the old videos
  2. Keep the non blocking lint as a compromise, which can be use in the transition period
  3. Once all the videos are subtitled, switch that lint to an hard error

@bfabio bfabio removed their assignment Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

No branches or pull requests

2 participants