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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ui): improve types #11085

Merged
merged 3 commits into from
Oct 26, 2021
Merged

Conversation

Shinigami92
Copy link
Contributor

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Documentation
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • It's submitted to the dev branch (or v[X] branch)
  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix: #xxx[,#xxx], where "xxx" is the issue number)
  • It's been tested on a Cordova (iOS, Android) app
  • It's been tested on a Electron app
  • Any necessary documentation has been added or updated in the docs (for faster update click on "Suggest an edit on GitHub" at bottom of page) or explained in the PR's description.

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

While starting to build an electron-quasar app, I experienced some lazy defined types and I want to improve the auto-completion in VSCode for TypeScript users like me.

https://github.com/Shinigami92/watchlist/blob/main/src/quasar.d.ts

So @yusufkandemir suggested that I can update these types by changing the json schema files.
This may be just the start from my side to make some contributions to Quasar 馃檶
Please feel free to push me in the right direction if I do anything wrong 馃檪

@Shinigami92
Copy link
Contributor Author

@IlCallo You wrote me on Discord (https://discord.com/channels/415874313728688138/807654245640962059/901927932237348886)

What I would like is to create types that are more enhanced.

So instead of this (JSDoc comments omitted):

export interface QOptionGroupProps {
  size?: string | undefined;
  modelValue: any;
  options?:
    | {
        label: string;
        value: any;
        disable?: boolean;
        [index: string]: any;
      }[]
    | undefined;
  name?: string | undefined;
  type?: "radio" | "checkbox" | "toggle" | undefined;
  color?: string | undefined;
  keepColor?: boolean | undefined;
  dark?: boolean | undefined;
  dense?: boolean | undefined;
  leftLabel?: boolean | undefined;
  inline?: boolean | undefined;
  disable?: boolean | undefined;
  "onUpdate:modelValue"?: (value: any) => void;
}

I would like to create types like this:

export interface QOptionGroupProps<Value = any> {
  size?: string | undefined;
  modelValue: Value;
  options?:
    | {
        label: string;
        value: Value;
        disable?: boolean;
        [index: string]: any;
      }[]
    | undefined;
  name?: string | undefined;
  type?: "radio" | "checkbox" | "toggle" | undefined;
  color?: string | undefined;
  keepColor?: boolean | undefined;
  dark?: boolean | undefined;
  dense?: boolean | undefined;
  leftLabel?: boolean | undefined;
  inline?: boolean | undefined;
  disable?: boolean | undefined;
  "onUpdate:modelValue"?: (value: Value) => void;
}

And this is just one example...

So what do I have to do to define such types?

@Shinigami92
Copy link
Contributor Author

Shinigami92 commented Oct 25, 2021

Another example for QTableProps

export interface QTableProps<
    Row extends Record<string, any> = any,
    K = Row extends Record<string, any> ? keyof Row : string,
    Field = K | ((row: Row) => any)
  > {
  // ...
  rows?: Row[] | undefined;
  rowKey?: string | ((row: Row) => string) | undefined;
  // ...
  columns?:
    | {
        // ...
        field: Field;
        // ...
        align?: "left" | "right" | "center";
        // ...
        sort?: (a: any, b: any, rowA: Row, rowB: Row) => number;
        // ...
        format?: (val: any, row: Row) => string;
        style?: string | ((row: Row) => string);
        classes?: string | ((row: Row) => string);
        // ...
      }[]
    | undefined;
  // ...
  // And many more can be enhanced here
}

I also would prefer to extract the columns type, so devs can directly use QTableColumn[]

Copy link
Member

@yusufkandemir yusufkandemir left a comment

Choose a reason for hiding this comment

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

馃憦

ui/src/components/option-group/QOptionGroup.json Outdated Show resolved Hide resolved
ui/src/components/option-group/QOptionGroup.json Outdated Show resolved Hide resolved
ui/src/components/option-group/QOptionGroup.json Outdated Show resolved Hide resolved
ui/src/components/option-group/QOptionGroup.json Outdated Show resolved Hide resolved
ui/src/components/option-group/QOptionGroup.json Outdated Show resolved Hide resolved
ui/src/components/option-group/QOptionGroup.json Outdated Show resolved Hide resolved
Co-authored-by: Yusuf Kandemir <yusuf.kandemir@outlook.com.tr>
Co-authored-by: Yusuf Kandemir <yusuf.kandemir@outlook.com.tr>
@Shinigami92
Copy link
Contributor Author

I think this PR is ready to get merged.
I can still open further PRs in the future whenever I encounter something I need to enhance 馃檪

Copy link
Member

@yusufkandemir yusufkandemir left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, nicely done 馃憦

@IlCallo IlCallo merged commit 5cbf651 into quasarframework:dev Oct 26, 2021
@Shinigami92 Shinigami92 deleted the improve-types branch October 26, 2021 12:59
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.

None yet

4 participants