Skip to content

Conversation

huangminghuang
Copy link

This PR upgrades Vite from v4 to v5 and modernizes the frontend tooling, including a migration to ESLint's flat
configuration. These changes resolve a significant number of linting errors, improve accessibility, and enhance the overall
maintainability of the frontend codebase.

Key Changes:

  • Vite 5 Upgrade: The primary change is the upgrade of the vite package from version 4.5.14 to 5.4.20. Associated
    dependencies like @sveltejs/kit, @sveltejs/adapter-auto, and @sveltejs/adapter-static have also been updated for
    compatibility.

  • ESLint Flat Configuration: The legacy .eslintrc.cjs file has been replaced with the new eslint.config.js flat
    configuration file. This aligns the project with modern ESLint standards.

  • Svelte Code Refinements:

    • Linting Fixes: A large number of linting errors have been addressed across all .svelte files. Many of these fixes were
      AI-assisted and should be reviewed.
    • Accessibility (A11y) Improvements: Click handlers on non-interactive elements (div, img) have been replaced with
      semantic elements to improve keyboard navigation and screen reader support.
    • Reactive Statement Refactoring: Several reactive blocks in components like Header.svelte and RetrievedTimestamp.svelte
      have been refactored to prevent infinite loops and improve state management.
    • Best Practices: Keyed {#each} blocks have been implemented for better list rendering performance and reliability.
  • Code Formatting: All JavaScript (.js), JSON (.json), and CSS (.css) files have been automatically reformatted using npm
    run format to ensure consistent coding style across the project. These changes are purely stylistic.

@mitza-oci mitza-oci requested a review from sonndinh September 19, 2025 15:21
Comment on lines +18 to +23
e2e: {
setupNodeEvents(on, config) {
// implement node event listeners here
},
baseUrl: 'http://localhost:8080'
}
Copy link
Member

Choose a reason for hiding this comment

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

Looks like tabs are used for indentation instead of spaces in these and the other places. I think spaces are preferred for consistent rendering across editors; the current code seems using spaces as well.

Copy link
Author

Choose a reason for hiding this comment

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

The tab is generated from npm run format -- --write and the format is defined in .prettierrc which I did not change.

// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
/* global cy, describe, beforeEach, it */
Copy link
Member

Choose a reason for hiding this comment

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

This line appears in multiple files but I'm not sure if it should be included in the comment.

Copy link
Author

Choose a reason for hiding this comment

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

This is used to silent lint errors. Without it, you get the following lint errors

$ npm run lint
> [email protected] lint
> prettier --check --plugin-search-dir=. . && eslint .

[warn] Ignored unknown option --plugin-search-dir=..
Checking formatting...
All matched files use Prettier code style!

/home/DDSPermissionsManager/frontend/cypress/e2e/action_intervals.cy.js
  17:1  error  'describe' is not defined    no-undef
  18:2  error  'beforeEach' is not defined  no-undef
  19:3  error  'cy' is not defined          no-undef
  20:3  error  'cy' is not defined          no-undef
  21:3  error  'cy' is not defined          no-undef
  22:3  error  'cy' is not defined          no-undef
  25:2  error  'it' is not defined          no-undef
  26:3  error  'cy' is not defined          no-undef
  28:3  error  'cy' is not defined          no-undef
  30:3  error  'cy' is not defined          no-undef
  32:3  error  'cy' is not defined          no-undef
  34:3  error  'cy' is not defined          no-undef
  36:3  error  'cy' is not defined          no-undef
  38:3  error  'cy' is not defined          no-undef
  42:3  error  'cy' is not defined          no-undef
  47:3  error  'cy' is not defined          no-undef
  49:3  error  'cy' is not defined          no-undef
  52:2  error  'it' is not defined          no-undef
  53:3  error  'cy' is not defined          no-undef
  55:3  error  'cy' is not defined          no-undef
  57:3  error  'cy' is not defined          no-undef
  59:3  error  'cy' is not defined          no-undef
  61:3  error  'cy' is not defined          no-undef

@@ -1,3 +1,4 @@
/* eslint-disable no-unused-vars */
Copy link
Member

Choose a reason for hiding this comment

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

Should this comment be deleted?

Copy link
Author

Choose a reason for hiding this comment

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

It is used for silent lint errors, without it

$ npm run lint

> [email protected] lint
> prettier --check --plugin-search-dir=. . && eslint .

[warn] Ignored unknown option --plugin-search-dir=..
Checking formatting...
All matched files use Prettier code style!

/home/DDSPermissionsManager/frontend/cypress.config.js
  18:19  error  'on' is defined but never used      no-unused-vars
  18:23  error  'config' is defined but never used  no-unused-vars

✖ 2 problems (2 errors, 0 warnings)

padding: 0;
margin: 0;
cursor: pointer;
}

Choose a reason for hiding this comment

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

I'm seeing this in a lot of the components inside lib, could this be moved to a global stylesheet?

Comment on lines 18 to 19
font-family: 'Roboto Flex'; /*You can use whatever name that you want*/
src: url('../src/fonts/RobotoFlex-Variable.ttf');

Choose a reason for hiding this comment

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

While there's no issue with serving a locally hosted font, might be nice to consider https://fonts.google.com/specimen/Roboto+Flex as this could be cached in users browsers already and it would eliminate an additional call.

Copy link

@jackkeller jackkeller left a comment

Choose a reason for hiding this comment

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

Overall looks good, I would opt to remove a lot of the css duplication like .icon-button and similar repeated things and move them perhaps into frontend/src/app.css.

…l styles; update font-face to use Google Fonts
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 this pull request may close these issues.

3 participants