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

feat: add tailwind showcase component. #778

Merged
merged 11 commits into from
Nov 3, 2022

Conversation

javieri-empathy
Copy link
Contributor

Adds showcase components for the tailwind plugin.

  • Updates vite to version 3
  • Generates a new entry point for tailwind plugin: @empathyco/x-tailwindcss/showcase with the showcase components
  • Configures build of this new entry point with rollup, as with Vite I found impossible to generate proper types in library mode.

content: [
'./public/index.html',
'./src/**/*.vue',
'./node_modules/@empathyco/x-tailwindcss/showcase/**/*.js'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have to be very careful with this if moved to the archetype. With this line it will include the full set of components CSS. It should only be included for development purposes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Different configs for dev and for build can be a solution for this:
tailwindlabs/tailwindcss#9405

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is another proposal that I thought yesterday (yeah, I know, too late) about exposing a dev server rather than a set of Vue components.
In my mind this would allow us to remove the rollup config for the showcase components, it would facilitate using this from a custom project regardless the components library used, and also ease up this configuration (run a command instead of setting up a view and a development tailwind config).
https://searchbroker.atlassian.net/browse/EX-7329

Copy link
Contributor

Choose a reason for hiding this comment

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

I love how that sounds Javi :sisi3:

@javieri-empathy javieri-empathy marked this pull request as ready for review October 13, 2022 14:30
@javieri-empathy javieri-empathy requested a review from a team as a code owner October 13, 2022 14:30
@diegopf diegopf self-requested a review October 31, 2022 09:59
import { Vue, Component, Prop } from 'vue-property-decorator';

@Component
export default class XdsButtonShowcase extends Vue {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering whether it makes sense or not to have a mixin covering all the logic that may be shared between different showcase components in the future. In the end I understand that sizes, colors, etc. apply to most of them, so my suggestion is something like this:


import { Vue, Component, Prop } from 'vue-property-decorator';

@Component
export default class XdsComponentShowcaseMixin extends Vue {
  @Prop({ required: true })
  public baseClass!: string;

  @Prop({ default: () => ['xs', 'sm', 'md', 'lg', 'xl'] })
  public sizes!: string[];

  @Prop({
    default: () => [
      'neutral',
      'lead',
      'auxiliary',
      'accent',
      'highlight',
      'success',
      'warning',
      'error'
    ]
  })
  public colors!: string[];

  @Prop()
  public combinations!: string[];

  protected get sections(): Record<string, string[]> {
    return {
      Default: [this.baseClass],
      Colors: this.colors.map(this.prefixWith(this.baseClass)),
      Sizes: this.sizes.map(this.prefixWith(this.baseClass)),
      ...(this.combinations && {
        Combinations: this.combinations.map(this.prefixWith(this.baseClass))
      })
    };
  }

  protected prefixWith(prefix: string): (value: string) => string {
    return cssClass => `${prefix} ${prefix}-${cssClass}`;
  }
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I think @alvarodE comment makes sense. Another option is to create a generic component that receives via props the configuration. The showcase component could get the tailwind config via resolveConfig so we have the prefix and colors. Afterwards a generic xds-component could just paint the given config. Something like

// Showcase component
    mounted(): void {
      const config = resolveConfig(this.tailwindConfigFile);
      this.colors = config.theme?.colors ?? {};
      this.prefix = config.prefix ??  '';
    }
  <div>
    <h3>{{ componentConfig.tag }}</h3>
    <ul>
      <li v-for="(_, colorName) in tailwindConfig.colors" :key="colorName">
        <component
          :is="componentConfig.tag"
          :class="[
            `${tailwindConfig.prefix}${componentConfig.base}-${colorName}`,
            componentConfig.classes
          ]"
        >
          {{ colorName }}
        </component>
      </li>
    </ul>
  </div>
</template>

<script lang="ts">
  import { Component, Prop, Vue } from 'vue-property-decorator';
  @Component
  export default class XdsItem extends Vue {
    @Prop({
      required: true
    })
    public tailwindConfig!: {
      prefix: string;
      colors: Record<string, object>;
    };
    @Prop({
      required: true
    })
    public componentConfig!: {
      tag: string;
      base: string;
      classes: string;
    };
  }
</script>

In the example above only colors are displayed but I think the point is clear 😅

Copy link
Contributor Author

@javieri-empathy javieri-empathy Nov 2, 2022

Choose a reason for hiding this comment

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

I thought about those 2 approaches. But it is something I decided not to do in this PR because of early abstractions. I don't have a clear visibility of what would be helpful for each component that we design. So until we have 2-3 more components I wouldn't abstract the utilities to generate this showcase.

Copy link
Contributor

@diegopf diegopf left a comment

Choose a reason for hiding this comment

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

Good job! As we discussed with @tajespasarela earlier I think it could be fine to merge this approach and just rethink it afterwards when we need to include another component in the showcase.

packages/x-tailwindcss/tsconfig.json Outdated Show resolved Hide resolved
import { Vue, Component, Prop } from 'vue-property-decorator';

@Component
export default class XdsButtonShowcase extends Vue {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think @alvarodE comment makes sense. Another option is to create a generic component that receives via props the configuration. The showcase component could get the tailwind config via resolveConfig so we have the prefix and colors. Afterwards a generic xds-component could just paint the given config. Something like

// Showcase component
    mounted(): void {
      const config = resolveConfig(this.tailwindConfigFile);
      this.colors = config.theme?.colors ?? {};
      this.prefix = config.prefix ??  '';
    }
  <div>
    <h3>{{ componentConfig.tag }}</h3>
    <ul>
      <li v-for="(_, colorName) in tailwindConfig.colors" :key="colorName">
        <component
          :is="componentConfig.tag"
          :class="[
            `${tailwindConfig.prefix}${componentConfig.base}-${colorName}`,
            componentConfig.classes
          ]"
        >
          {{ colorName }}
        </component>
      </li>
    </ul>
  </div>
</template>

<script lang="ts">
  import { Component, Prop, Vue } from 'vue-property-decorator';
  @Component
  export default class XdsItem extends Vue {
    @Prop({
      required: true
    })
    public tailwindConfig!: {
      prefix: string;
      colors: Record<string, object>;
    };
    @Prop({
      required: true
    })
    public componentConfig!: {
      tag: string;
      base: string;
      classes: string;
    };
  }
</script>

In the example above only colors are displayed but I think the point is clear 😅

packages/x-components/tailwind.config.js Show resolved Hide resolved
packages/x-tailwindcss/tsconfig.json Outdated Show resolved Hide resolved
packages/x-tailwindcss/rollup.config.js Outdated Show resolved Hide resolved
packages/x-tailwindcss/package.json Show resolved Hide resolved
@alvarodE
Copy link
Contributor

alvarodE commented Nov 2, 2022

With the last merge of main into this branch there are a lot of changes on the doc files that I think we should discard. Is it because of the Prettier or something? @javieri-empathy

@javieri-empathy
Copy link
Contributor Author

Fuck, I can promise you I didn't format them intentionally. I'm looking at them and I can tell you they some of them were not properly formatted, but there are also some changes that do not make sense, like moving titles to the previous line. My bet is that when merging main branch changes prettier decided to format them.

…lwind-showcase

# Conflicts:
#	packages/x-tailwindcss/demo/index.html
#	packages/x-tailwindcss/tsconfig.json
@javieri-empathy javieri-empathy force-pushed the feature/EX-7088-x-tailwind-showcase branch from 7fd3c2a to 56ea836 Compare November 3, 2022 08:03
@herrardo herrardo merged commit 0178ef5 into main Nov 3, 2022
@tajespasarela tajespasarela deleted the feature/EX-7088-x-tailwind-showcase branch November 7, 2022 07:55
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.

5 participants