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/グループ新規作成のレイアウトを Figma に揃える #293

Merged
merged 4 commits into from
Sep 24, 2024

Conversation

anko9801
Copy link
Contributor

@anko9801 anko9801 commented Sep 13, 2024

User description

fix: #166


PR Type

Enhancement


Description

  • InputTextareaコンポーネントをMarkdownTextareaに置き換え、説明入力を改善しました。
  • Figmaデザインに合わせてレイアウトとスタイリングを調整しました。
  • フォームラベルにfont-mediumクラスを追加し、一貫性を持たせました。
  • グループ作成ボタンのテキストを簡略化しました。

Changes walkthrough 📝

Relevant files
Enhancement
NewGroupPage.vue
Update New Group Page layout and components                           

src/pages/NewGroupPage.vue

  • InputTextarea replaced with MarkdownTextarea for description input.
  • Adjusted layout and styling for better alignment with Figma design.
  • Updated form labels to use font-medium class for consistency.
  • Simplified button text for group creation.
  • +24/-24 

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link

    github-actions bot commented Sep 13, 2024

    PR Reviewer Guide 🔍

    (Review updated until commit ae4164a)

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    コード整合性
    MarkdownTextareaコンポーネントの導入により、入力フィールドの一貫性が向上していますが、placeholder属性が空です。ユーザーが何を入力すべきかのヒントを提供するために、適切なプレースホルダーテキストを追加することをお勧めします。

    スタイルの一貫性
    InputNumberInputSelectMultipleclass属性にmr-2w-fullが追加されていますが、他の類似コンポーネントとのスタイルの一貫性を確認することが重要です。特に、他の入力フィールドにも同様のスタイリングが適用されているかを確認してください。

    @reiroop
    Copy link
    Contributor

    reiroop commented Sep 14, 2024

    /review

    @reiroop
    Copy link
    Contributor

    reiroop commented Sep 14, 2024

    /improve

    Copy link

    github-actions bot commented Sep 14, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    SimpleButton のスタイル属性を class に統合してください。

    SimpleButton コンポーネントの font-sizepadding 属性が不正確な値を持っています。Vue.js では、これらの属性を style
    または class を通じて適切に設定する必要があります。

    src/pages/NewGroupPage.vue [89-96]

     <SimpleButton
    -  class="ml-auto mt-8 block"
    +  class="ml-auto mt-8 block text-xl p-4"
       :disabled="isSending"
    -  font-size="xl"
    -  padding="md"
       @click="handleCreateGroup">
       グループを作成
     </SimpleButton>
     
    Suggestion importance[1-10]: 9

    Why: Correcting the way styles are applied to the SimpleButton component by using class instead of incorrect attributes ensures consistency and follows best practices in Vue.js, which is crucial for maintainability.

    9
    Enhancement
    MarkdownTextareaplaceholder 属性に説明テキストを追加してください。

    MarkdownTextarea コンポーネントの placeholder
    属性が空です。ユーザーが入力する際のガイドとして、適切なプレースホルダーテキストを設定することをお勧めします。

    src/pages/NewGroupPage.vue [56-60]

     <MarkdownTextarea
       id="details"
       v-model="group.description"
    -  placeholder=""
    +  placeholder="グループの説明を入力"
       required />
     
    Suggestion importance[1-10]: 8

    Why: Adding a placeholder text to the MarkdownTextarea component improves user experience by providing guidance on what to input, which is a significant enhancement.

    8
    Responsive design
    InputNumber のマージンをレスポンシブに対応させてください。

    InputNumber コンポーネントの class 属性に mr-2
    のみが設定されていますが、レスポンシブデザインを考慮して、他のブレークポイントでも適切なマージンが適用されるようにすることをお勧めします。

    src/pages/NewGroupPage.vue [65-69]

     <InputNumber
       v-model="group.budget"
    -  class="mr-2"
    +  class="mr-2 md:mr-4"
       :min="1"
       required />円
     
    Suggestion importance[1-10]: 6

    Why: Making the margin responsive for the InputNumber component is a good practice for better design adaptability across different screen sizes, though it is a minor improvement.

    6

    Copy link

    Persistent review updated to latest commit ae4164a

    Copy link
    Contributor

    @reiroop reiroop left a comment

    Choose a reason for hiding this comment

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

    must レスポンシブ対応お願いします...!issueにするのでも大丈夫です!

    @reiroop reiroop self-requested a review September 21, 2024 10:12
    Copy link
    Contributor

    @reiroop reiroop left a comment

    Choose a reason for hiding this comment

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

    ごめんなさい、間違えました!LGTMです、ありがとうございます
    nits こちらも#268 と同じように、画面幅が390pxを切ったあたりから左右にスクロールできるようになってしまうのと、全体が画面の左端と接してしまうのが少し気になるかなーって感じです。今のままでも全然okです!

    @anko9801 anko9801 merged commit bb4b93d into main Sep 24, 2024
    6 checks passed
    @anko9801 anko9801 deleted the feat/new_group branch September 24, 2024 12:58
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    グループ作成ページ
    2 participants