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

Implement removal and linting of native event name usage #1603

Closed
8 tasks done
JoCa96 opened this issue Jul 23, 2024 · 2 comments
Closed
8 tasks done

Implement removal and linting of native event name usage #1603

JoCa96 opened this issue Jul 23, 2024 · 2 comments
Assignees
Labels
dev Requires technical expertise

Comments

@JoCa96
Copy link
Collaborator

JoCa96 commented Jul 23, 2024

Why?

Currently, we are re-emitting events like click or change.
This creates complications and confusion for our users:

  • the emit names are already reserved by native HTML events
  • the emit payload is not compatible
  • the custom emits don't work with Vue event and key modifiers
  • there is a performance overhead of listening for the native events and re-emitting them
  • native events don't behave like Vue events (bubbling, etc.)

Acceptance criteria

  • all onyx emits with native event names have been removed or renamed
  • a lint rule enforcing non-native event/emit name collisions is added. (can be a follow-up ticket)
  • a storybook helper/util is provided that allows relevant events to be logged

Definition of Done

  • follow-up tickets were created if necessary
  • updated version + documentation is deployed
  • Storybook can show the feature
  • Storybook code snippet of new/changed examples are checked that they are generated correctly

Approval

  1. Go To https://storybook.onyx.schwarz/?path=/story/form-input--default
  2. Open the "add-on sidebar" if necessary
  3. Scroll to the Relevant HTML events section and you will find the documented elements

Implementation details

Make use of eslint-plugin-vue/lib/utils:

const utils = require("eslint-plugin-vue/lib/utils");

Reference implementations

See the following as examples for similar lint rules:

@JoCa96 JoCa96 self-assigned this Jul 23, 2024
@mj-hof mj-hof added the dev Requires technical expertise label Jul 26, 2024
@mj-hof mj-hof added this to the Product improvements milestone Jul 26, 2024
@MajaZarkova MajaZarkova self-assigned this Aug 26, 2024
@MajaZarkova
Copy link
Contributor

  • PR open
  • The PR doesn't have a changeset yet
  • A follow up ticket should be created regarding this comment
  • There is a replay on the other two comments with the issues I have encountered.
  • The last point of this ticket (the part in Implementation details) is not implemented

@JoCa96 JoCa96 self-assigned this Sep 16, 2024
@JoCa96
Copy link
Collaborator Author

JoCa96 commented Sep 16, 2024

Issue with eslint-plugin-vue: vuejs/eslint-plugin-vue#2557

JoCa96 added a commit that referenced this issue Sep 17, 2024
…#1799)

Relates to #1603 

Remove/rename native event names on every component

---------

Co-authored-by: Jonathan Carle <jonathan_leo.carle@mail.schwarz>
Co-authored-by: Lars Rickert <lars.rickert@mail.schwarz>
JoCa96 added a commit that referenced this issue Sep 17, 2024
…lint-plugin-vue" until its released (#1877)

Duplicate "no-shadow-native-events" from "eslint-plugin-vue" until it is
released:
- PR with `eslint-plugin-vue`:
vuejs/eslint-plugin-vue#2558
  - Tests can also be found in that PR
- Official Rule proposal:
vuejs/eslint-plugin-vue#2557

Relates to #1603
JoCa96 added a commit that referenced this issue Sep 18, 2024
A new Storybook Util `withNativeEventLoggingFor` is introduced.
It generates Storybook `ArgTypes` that allow us to document and log
relevant native events in Storybook.
These can be merged with other `ArgTypes`.
The native events will be documented in a new section "Relevant HTML
Events".

Relates to #1603

---------

Co-authored-by: Lars Rickert <lars.rickert@mail.schwarz>
@JoCa96 JoCa96 closed this as completed Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev Requires technical expertise
Projects
Status: Done
Development

No branches or pull requests

3 participants