Skip to content

Comments

use sentence case for button labels#25

Open
donatPeter wants to merge 1 commit intomasterfrom
sentence_case
Open

use sentence case for button labels#25
donatPeter wants to merge 1 commit intomasterfrom
sentence_case

Conversation

@donatPeter
Copy link
Member

No description provided.

@donatPeter donatPeter requested review from Copilot and getamas June 23, 2025 08:22
@donatPeter donatPeter self-assigned this Jun 23, 2025
@netlify
Copy link

netlify bot commented Jun 23, 2025

Deploy Preview for gs-components ready!

Name Link
🔨 Latest commit ac96ac4
🔍 Latest deploy log https://app.netlify.com/projects/gs-components/deploys/68590ee55e37da0008517124
😎 Deploy Preview https://deploy-preview-25--gs-components.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 project configuration.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces sentence-case formatting for button labels by adding a helper in the component and updating Storybook stories to use raw slot content through render functions.

  • Added a toSentenceCase function in GsButton.vue and applied it to the default slot; removed the CSS text-transform: capitalize.
  • Converted Button.stories.ts stories (Default, Disabled, Loading) to use render functions with raw slot labels.
  • Updated the “Small Size” label to uppercase in the Sizes story to showcase the sentence-case helper.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/components/GsButton.vue Added toSentenceCase helper, applied in template, removed CSS text-transform.
src/stories/Button.stories.ts Switched Default/Disabled/Loading to render-based templates and adjusted labels.
Comments suppressed due to low confidence (6)

src/components/GsButton.vue:65

  • Add unit tests for toSentenceCase to cover edge cases like empty strings, punctuation, and mixed-case inputs to ensure consistent behavior.
const toSentenceCase = (str: string): string => {

src/stories/Button.stories.ts:73

  • The slot content is hard-coded, so Storybook controls for args.default won’t work. Use {{ args.default }} inside the template to bind the text dynamically, e.g.: <GsButton v-bind="args">{{ args.default }}</GsButton>.
      <GsButton v-bind="args">Button</GsButton>`,

src/stories/Button.stories.ts:140

  • Similar to the Default story, this hard-codes the label. Replace with <GsButton v-bind="args">{{ args.default }}</GsButton> so the disabled label can be controlled via args.
      <GsButton disabled v-bind="args">Disabled</GsButton>`,

src/stories/Button.stories.ts:151

  • Hard-coding the loading label prevents Storybook text controls. Switch to <GsButton v-bind="args">{{ args.default }}</GsButton> to keep the label configurable.
      <GsButton loading v-bind="args">Loading</GsButton>`,

src/components/GsButton.vue:87

  • Casting slot content directly to string may cause runtime errors if the slot output isn’t a plain string. Consider checking typeof children === 'string' or providing a fallback before calling toSentenceCase.
      {{ toSentenceCase($slots.default?.()[0]?.children as string) }}

src/components/GsButton.vue:86

  • [nitpick] The sentence-case CSS class isn’t defined anywhere. If it’s not needed for styling, consider removing it to avoid confusion.
    <span v-if="$slots.default" class="text sentence-case">

</v-icon>
<span v-if="$slots.default" class="text">
<slot />
<span v-if="$slots.default" class="text sentence-case">
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would use only css to achieve this one, I think there is no need to make it such a complex logic for a simple case. Its the responsibility of the person who adds the copy for the button to not make it all uppercase. But if we really need to take that into account that can be achieved with css having the button element styled to lowercase and the text class to first letter uppercase as in the example below.

There is no need for checking the template thats injected into the slot because the span wrapper element will handle the casing regardless of the inner content.

.text:first-letter {
    text-transform: uppercase;
}

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