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: Component for tag with story #9

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Conversation

nithindavid
Copy link
Contributor

image

Copy link

netlify bot commented Jan 24, 2024

Deploy Preview for chatwoot-design ready!

Name Link
🔨 Latest commit fd08d35
🔍 Latest deploy log https://app.netlify.com/sites/chatwoot-design/deploys/65b76df1c072c90008f4d340
😎 Deploy Preview https://deploy-preview-9--chatwoot-design.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@nithindavid nithindavid changed the title feat: Component for tag component feat: Component for tag with story Jan 25, 2024
});

const slots = useSlots();
let containerClasses = '';
Copy link
Member

Choose a reason for hiding this comment

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

We can declare containerClass as an array, and then use containerClasses.push('text-xs', 'h-5', 'min-w-[20px]', 'py-0.5', 'rounded') instead of string concatenation

Comment on lines 19 to 50
if (props.size === 's') {
containerClasses += ' text-xs h-5 min-w-[20px] py-0.5 rounded ';

// Variant: SYMBOL-TEXT
if (hasSymbol && hasContent && !hasAction) {
containerClasses += 'pr-1 ';
}
// Variant: TEXT-ACTION
else if (!hasSymbol && hasContent && hasAction) {
containerClasses += 'pl-1 ';
}

// Variant: TEXT
if (hasContent) {
containerClasses += 'px-1 ';
}
} else if (props.size === 'm') {
containerClasses += ' text-sm h-6 min-w-[24px] leading-4 p-1 rounded-md ';

// Variant: SYMBOL-TEXT
if (hasSymbol && hasContent && !hasAction) {
containerClasses += 'pr-2 ';
}
// Variant: TEXT-ACTION
else if (!hasSymbol && hasContent && hasAction) {
containerClasses += 'pl-2 ';
}
// Variant: TEXT
else if (!hasSymbol && hasContent && !hasAction) {
containerClasses += 'px-2 ';
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can wrap this in a computed, or a watchEffect the style will become reactive

const containerClasses = computed(() => {
  let classes = [];

  if (props.size === 's') {
    classes.push('text-xs', 'h-5', 'min-w-[20px]', 'py-0.5', 'rounded');

    // Variant: SYMBOL-TEXT
    if (hasSymbol && hasContent && !hasAction) {
      classes.push('pr-1');
    }
    // Variant: TEXT-ACTION
    else if (!hasSymbol && hasContent && hasAction) {
      classes.push('pl-1');
    }

    // Variant: TEXT
    if (hasContent) {
      classes.push('px-1');
    }
  } else if (props.size === 'm') {
    classes.push('text-sm', 'h-6', 'min-w-[24px]', 'leading-4', 'p-1', 'rounded-md');

    // Variant: SYMBOL-TEXT
    if (hasSymbol && hasContent && !hasAction) {
      classes.push('pr-2');
    }
    // Variant: TEXT-ACTION
    else if (!hasSymbol && hasContent && hasAction) {
      classes.push('pl-2');
    }
    // Variant: TEXT
    else if (!hasSymbol && hasContent && !hasAction) {
      classes.push('px-2');
    }
  }

  return classes.join(' ');
});

ghost: 'outline-transparent',
outlined: 'outline-slate-500',
};
const emits = defineEmits(['action', 'click']);
Copy link
Member

Choose a reason for hiding this comment

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

We can type the emits

const emit = defineEmits<{
  action: []
  click: []
}>()

Also we should probably forward the event object too

Comment on lines 56 to 58
const disabledClasses = props.disabled ? 'opacity-50' : '';
const disabledActionClasses = props.disabled ? 'cursor-default' : 'hover:bg-slate-400';
const clickableClasses = props.clickable ? 'cursor-pointer hover:bg-slate-300 outline-slate-300' : '';
Copy link
Member

Choose a reason for hiding this comment

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

We should move this to the template itself, using the object syntax, that way tailwind intellisense will work for them in IDEs

class="w-4 h-4 inline-block border-[3px] border-white rounded-md"
:style="{ background: props.color }"
></span>
<span v-else-if="props.icon" class="w-4 h-4 inline-block" :class="props.icon"></span>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<span v-else-if="props.icon" class="w-4 h-4 inline-block" :class="props.icon"></span>
<span v-else-if="props.icon" class="w-4 h-4 inline-block" :class="props.icon"/>

<span
v-if="props.color"
class="w-4 h-4 inline-block border-[3px] border-white rounded-md"
:style="{ background: props.color }"
Copy link
Member

Choose a reason for hiding this comment

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

Will these colors not be standard?

Comment on lines +4 to +24
export type NonMutuallyExclusiveProps = {
size?: TagSizes;
icon?: string;
emoji?: string;
color?: string;
style?: TagStyles;
disabled?: boolean;
clickable?: boolean;
removable?: boolean;
expandable?: boolean;
};

export type TagProps = NonMutuallyExclusiveProps &
(
| { emoji?: string; icon?: undefined; color?: undefined }
| { emoji?: undefined; icon?: string; color?: undefined }
| { emoji?: undefined; icon?: undefined; color?: string }
) & (
| { removable?: boolean, expandable?: undefined }
| { removable?: undefined, expandable?: boolean }
)
Copy link
Member

Choose a reason for hiding this comment

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

Just a small cosmetic suggestion, makes it more readable. Feel free to ignore it

export type BaseTagProps = {
  size?: TagSizes;
  style?: TagStyles;
  disabled?: boolean;
  clickable?: boolean;
};

export type EmojiTagProps = BaseTagProps & {
  emoji: string;
  icon?: never;
  color?: never;
  removable?: boolean;
  expandable?: never;
};

export type IconTagProps = BaseTagProps & {
  emoji?: never;
  icon: string;
  color?: never;
  removable?: never;
  expandable?: boolean;
};

export type ColorTagProps = BaseTagProps & {
  emoji?: never;
  icon?: never;
  color: string;
  removable?: boolean;
  expandable?: boolean;
};

export type TagProps = EmojiTagProps | IconTagProps | ColorTagProps;

Comment on lines 56 to 58
const disabledClasses = props.disabled ? 'opacity-50' : '';
const disabledActionClasses = props.disabled ? 'cursor-default' : 'hover:bg-slate-400';
const clickableClasses = props.clickable ? 'cursor-pointer hover:bg-slate-300 outline-slate-300' : '';
Copy link
Member

Choose a reason for hiding this comment

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

If I have set disabled and clickable both to true, the UI will show cursor-pointer right?

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.

2 participants