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

ソート時にtoSorted()を使用 #258

Merged
merged 1 commit into from
Dec 11, 2024
Merged

ソート時にtoSorted()を使用 #258

merged 1 commit into from
Dec 11, 2024

Conversation

ogu-kazemiya
Copy link
Contributor

@ogu-kazemiya ogu-kazemiya commented Dec 10, 2024

User description

#255 で指摘を受けた点の修正です


PR Type

Enhancement


Description

  • computedプロパティsortedMembersを修正し、toSorted()メソッドを使用してメンバーを加入日昇順にソート。
  • ソートロジックをyearsemesterに基づいて実装。
  • v-forディレクティブでmembersの代わりにsortedMembersを使用することで、ソートされたリストをテンプレートに表示。

Changes walkthrough 📝

Relevant files
Enhancement
MemberList.vue
メンバーリストを加入日昇順にソートするロジックの修正                                                             

src/components/Project/MemberList.vue

  • computedプロパティsortedMembersを修正し、toSorted()メソッドを使用してメンバーを加入日昇順にソート。
  • ソートロジックをyearsemesterに基づいて実装。
  • +1/-2     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @ogu-kazemiya ogu-kazemiya requested a review from Pugma December 10, 2024 00:32
    Copy link

    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    255 - Fully compliant

    Fully compliant requirements:

    • プロジェクトメンバーを加入日昇順にソートするためのロジックを追加する。
    • computedプロパティsortedMembersを作成し、yearsemesterを基準にメンバーをソート。
    • テンプレート内でv-forディレクティブをsortedMembersに変更し、ソートされたリストを表示。
    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Performance Issue
    toSorted()メソッドのパフォーマンスとメモリ使用量を確認し、大量のメンバーデータを扱う場合の効率性を検討する必要があります。

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    toSorted()sort() に置き換えて、JavaScript の標準メソッドを使用するように修正します。

    toSorted() メソッドは JavaScript の標準メソッドではありません。sort() メソッドを使用して、配列をソートしてください。

    src/components/Project/MemberList.vue [15-19]

    -const li = props.members.toSorted((a, b) => {
    +const li = props.members.sort((a, b) => {
       if (a.duration.since.year !== b.duration.since.year) {
         return a.duration.since.year - b.duration.since.year
       } else {
         return a.duration.since.semester - b.duration.since.semester
    Suggestion importance[1-10]: 10

    Why: The method toSorted() is not a standard JavaScript method, and using sort() correctly addresses the functionality intended in the PR, ensuring the code is executable and correct.

    10

    Copy link
    Contributor

    @sh0go07 sh0go07 left a comment

    Choose a reason for hiding this comment

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

    良さそうです!ありがとう!

    @ogu-kazemiya ogu-kazemiya merged commit dcfed88 into main Dec 11, 2024
    12 checks passed
    @ogu-kazemiya ogu-kazemiya deleted the fix/sort-members branch December 11, 2024 02:49
    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