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

No Issue but... #4

Open
gearsdigital opened this issue Jan 20, 2025 · 1 comment
Open

No Issue but... #4

gearsdigital opened this issue Jan 20, 2025 · 1 comment

Comments

@gearsdigital
Copy link

I was looking for a solution to an issue in an upcoming project when I stumbled upon dynamowaves. I just want to say thank you! This is truly an awesome custom element integration and I like that It has a great API and excellent documentation.

Why did you decide to use data-*-attributes for configuration? Is there a specific reason @mzebley? The component API might look a bit cleaner using regular attributes.

<dynamo-wave
   animate="true"
   wave-points="4"
   wave-variance="3"
></dynamo-wave>

This would also allow to make use of the attributeChangedCallback();

class MyCustomElement extends HTMLElement {
  static observedAttributes = ["animate"];

  constructor() {
    super();
  }

  attributeChangedCallback(name, oldValue, newValue) {
    console.log(
      `Attribute ${name} has changed from ${oldValue} to ${newValue}.`,
    );
  }
}

customElements.define("my-custom-element", MyCustomElement);

It might also be useful to utilize the Shadow DOM API to provide better encapsulation in component based architectures.

constructor() {
    this.attachShadow({ mode: 'open' });
}

I would be able to provide an MR for the aforementioned improvements if you consider to adapt some suggestions. Let me know.... Once again - Thank you!

@mzebley
Copy link
Owner

mzebley commented Jan 27, 2025

Thanks for checking out Dynamowaves and for the feedback!

Maintaining the data- prefix was actually a backwards compatibility choice - I wanted the v2 updates to work smoothly with existing implementations and not require having to run through and update the HTML. Really like your suggestion about regular attributes though! The attributeChangedCallback() approach would definitely clean up the API.

Not using Shadow DOM was intentional - I lean heavily on global classes and realtime CSS tokens updates in the design system I've been putting together, and I wanted those styles to propagate cleanly down to the Dynamowave so that it fits within its environment. The element itself doesn't contain any CSS either, so there's no worry of cross contamination or styles leaking out.

I'd be super interested in seeing your attribute-based API updates. Maybe we could explore it as an opt-in feature or plan it for a future release?

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

No branches or pull requests

2 participants