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

Component and option type definitions + code docs #390

Merged
merged 5 commits into from
Jul 12, 2023

Conversation

mauromascarenhas
Copy link

@mauromascarenhas mauromascarenhas commented Jun 21, 2023

Proposed changes

Apply type definition for every Materialize component (I have taken some from DefinitelyTyped, since I'm the maintainer of @materializecss/materialize types).

  • Base Component is a generic class with "BaseOptions" interface:
    • Declared function overloades for component initializer;
    • Declared "virtual" methods and given a proper documentation for them.
  • Every component (except by Toast) inherit from Component and their respective options from BaseOptions;
  • Removed "circular" dependencies (check "Breaking changes");
  • Added missing types to event handlers and their respective args;
  • Component options is no longer "protected" (the user should be able to check the initialization options as well, just as mentioned in the docs);
  • Fixed:
    • Undefined instance reference in Collapsible "destructor";
    • Touch event handler when target does not have a close carousel item;
    • Type definitions using "Boolean" object as type.

Breaking changes

  • Completely removed native support to jQuery;
  • Unused methods, such as M::objectSelectorString, have been removed;
  • Global utility functions and variables have been migrated to "Utils" module, which is available under "M.Utils" (this is an important change, since it removes circular dependency from the component sources 😃).

Affected PRs:

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to change).

Checklist:

  • I have read the CONTRIBUTING document.
  • My commit messages follows the conventional commit format
  • My change requires a change to the documentation, and updated it accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

- Added type definition for generic component (base class);
- Added type definition for static and virtual class methods;
- Added type definitions for object options;
- Added overload definitions to "init" methods;
- Added initial version for code docs (props);
- Dropped support to jQuery selector in "getInstance" method;
- Fixed "Collapsible" destructor.
- Added type definition for generic component (base class);
- Added type definition for static and virtual class methods;
- Added type definitions for object options;
- Added overload definitions to "init" methods;
- Added initial version for code docs (props);
- Dropped support to jQuery selector in "getInstance" method;
- Fixed "Collapsible" destructor.
- Finished removing native jQuery support;
- Migrated global constants and utilitary functions to "Utils" module;
- Added type definitions and docs to "Utils" module;
- Exposed "Utils" module in global namespace ("M");
- Components no longer need to import global namespace;
- Removed unnecessary throttle wrapper function on components;
- Updated type definition for toast function + added docs;
- Fix touch event when target does not have a close carousel item;
- Fixed incorrect event instance check;
- Fixed type definitions using "Boolean" object as type.
@danice
Copy link

danice commented Jun 21, 2023

Great work!
One thing: I have tested the changes in this test project an it gives errors for the init method:

imagen

The reason is that querySelectorAll returns a NodeListOf<Element> instead of NodeListOf<HTMLElement>.
One possible solution to avoid having to add this querySelectorAll<HTMLElement> all the time would be changing the init parameters to

static init(els: InitElements<Element>, options: Partial<CarouselOptions>): Carousel[];

instead of the

static init(els: InitElements<HTMLElement>, options: Partial<CarouselOptions>): Carousel[];

What do you think?

@mauromascarenhas
Copy link
Author

mauromascarenhas commented Jun 21, 2023

Great work! One thing: I have tested the changes in this test project an it gives errors for the init method:

imagen

The reason is that querySelectorAll returns a NodeListOf<Element> instead of NodeListOf<HTMLElement>. One possible solution to avoid having to add this querySelectorAll<HTMLElement> all the time would be changing the init parameters to

static init(els: InitElements<Element>, options: Partial<CarouselOptions>): Carousel[];

instead of the

static init(els: InitElements<HTMLElement>, options: Partial<CarouselOptions>): Carousel[];

What do you think?

Well, I haven't noticed it. Thanks for reporting.
I'll replace this type requirement in initialization functions and constructors.

UPDATE

From what I've seen, this issue only applies to "NodeList". Therefore, I'm just updating the "init" function overload types.

- New "MElement" type (HTMLElement | Element);
- "init" overloads which supports nodelist must support "MElement" type;
- Made "options" an optional attribute in "init" functions;
- Update "init" types for input elements;
- Removed unnecessary imports.
@mauromascarenhas
Copy link
Author

@danice, I've made some changes to the initialization overloads which allows "Element" type to be considered as well 😄. The following snippets illustrates a picker being initialized either by a single element (even though it is typed as HTMLInputElement) or by a NodeListOf<Elements>.

Initialization samples of TimePicker

I've also updated component initializers so as to allow "empty" options 😃.

@wuda-io
Copy link
Member

wuda-io commented Jun 22, 2023

Haha good work! That will take some time to review 😆 I will check locally...

Merge 'v2-dev' into component-types-docs
@mauromascarenhas
Copy link
Author

I've just had to merge v2-dev into this branch, since #387 has been already merged, thus, requiring a small refactor, as mentioned in the topic.

@wuda-io
Copy link
Member

wuda-io commented Jul 12, 2023

Awesome! Great work 👍
I see you used some sort of documentation tool in your code. Is this TSDoc?

@wuda-io wuda-io merged commit 3f89544 into materializecss:v2-dev Jul 12, 2023
@wuda-io wuda-io mentioned this pull request Jul 12, 2023
@mauromascarenhas
Copy link
Author

It is JSDoc as well.
Since TS is a superset of JS, most of JSDoc tags also work with TS.

@mauromascarenhas mauromascarenhas deleted the component-types-docs branch July 12, 2023 14:06
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