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

refactor: introduce kebab events #526

Merged
merged 30 commits into from
Aug 31, 2021
Merged

refactor: introduce kebab events #526

merged 30 commits into from
Aug 31, 2021

Conversation

acstll
Copy link
Collaborator

@acstll acstll commented Aug 18, 2021

Testing a possible implementation with a utility function to transition from scaleChange (camel) to scale-change (kebab).

Because scaleChange does not work with Vue 3, and kebab-case is more "aligned" with custom elements semantics.

This is a big change but we're making it non-breaking.

It will fix #490

Components affected

  • checkbox
  • collapsible
  • data-grid
  • date-picker
  • dropdown (had "inconsistent scaleKeyDown, this is breaking-ish but OK)
  • input
  • menu-flyout
  • menu-flyout-list (not emitting for some reason)
  • modal
  • pagination
  • radio-button
  • rating-stars
  • slider
  • switch
  • tag
  • text-field
  • textarea

Todo list for every component emitting public events

  • emit both events with emitEvent util
  • document deprecation
  • update spec (if any) to check both events are emitted
  • update actions in Storybook (different PR afterwards)
  • add new "events" section in Storybook (how?)

Framework checklist

  • Vue 3 (@scale-change="handleChange")
  • Vue 2 (@scale-change="handleChange")
  • React (with proxy package, onScale-change={handleChange}, …ugly but welp)
  • Angular ((scale-change)="handleChange()")
  • Svelte (on:scale-change={handleChange})

Does this mean we will change our events again to fit another framework? Nope.

@acstll acstll added the refactor Code change label Aug 18, 2021
@render
Copy link

render bot commented Aug 18, 2021

@acstll
Copy link
Collaborator Author

acstll commented Aug 18, 2021

All frameworks in the list checked and working.

Anything against this @nowseemee @oddcelot ?

(Remember I only applied this to scale-checkbox to test the implementation, the rest of the components will follow…)

https://github.com/telekom/scale/pull/526/files 👀

@oddcelot
Copy link
Collaborator

very nice 🥙
this will be a bit awkward for the time until we deprecate this 😅
maybe we should find a way to highlight the deprecation in the docs table for storybook

# Conflicts:
#	packages/components/src/components/tag/tag.tsx
@acstll acstll marked this pull request as ready for review August 27, 2021 06:53
@acstll acstll requested a review from eeegor as a code owner August 27, 2021 06:53
@acstll
Copy link
Collaborator Author

acstll commented Aug 27, 2021

The Toggle Group is still using only camel, but we can handle that in #507 🙏

@acstll
Copy link
Collaborator Author

acstll commented Aug 27, 2021

yarn build in root fails because types and Angular (we need to fix this before merging) 🤓

src/directives/proxies.ts(4723,16): error TS1005: ',' expected.
src/directives/proxies.ts(4723,38): error TS1005: ',' expected.
src/directives/proxies.ts(4723,62): error TS1005: ',' expected.
src/directives/proxies.ts(4723,63): error TS1005: ',' expected.
src/directives/proxies.ts(4723,73): error TS1005: ';' expected.
src/directives/proxies.ts(4728,1): error TS1128: Declaration or statement expected.

@nowseemee nowseemee merged commit 600e2f4 into main Aug 31, 2021
@nowseemee nowseemee deleted the refactor/kebab-events branch August 31, 2021 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Code change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vue3 scale events…
4 participants