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

コンテストチームの複数表示に対応 #193

Merged
merged 1 commit into from
Jul 6, 2024

Conversation

Pugma
Copy link
Collaborator

@Pugma Pugma commented Jul 6, 2024

User description

確認お願いします〜

変更前
image

変更後のデザインは以下のとおりです
image

close #192


PR Type

Enhancement


Description

  • コンテストチームの複数表示に対応し、各チームの名前と結果を表示するように変更
  • コンテスト名を<h4>タグに変更し、視覚的な階層を明確化
  • CSSスタイルを微調整し、リンクのマージンと詳細情報のインデントを改善

Changes walkthrough 📝

Relevant files
Enhancement
ContestsContainer.vue
コンテストチームの複数表示とスタイル調整                                                                         

src/components/User/ContestsContainer.vue

  • コンテストチームの複数表示に対応
  • コンテスト名を<h4>タグに変更
  • CSSスタイルの微調整
+12/-5   

💡 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 Jul 6, 2024

PR Reviewer Guide 🔍

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

データ構造の変更:
コンテストのチームを複数表示するためのデータ構造が変更されていますが、この変更が他のコンポーネントに影響を与えないか確認が必要です。

HTML構造の変更:
<div>から<h4>への変更は視覚的な階層を明確化するためのものですが、スタイルの影響を再確認する必要があります。

Copy link

github-actions bot commented Jul 6, 2024

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Score
Best practice
HTML内での改行タグの使用を避け、CSSでのスタイリングに置き換える。

チーム名と結果を表示する際に、

タグの使用を避け、CSSのスタイルで管理することをお勧めします。これにより、スタイルの調整が容易になり、HTMLの構造もよりクリーンに保つことができます。

src/components/User/ContestsContainer.vue [24-25]

-<br />
-{{ team.result }}
+<span class="$style.result">{{ team.result }}</span>
 
Suggestion importance[1-10]: 8

Why: This suggestion improves the separation of concerns by moving presentation details to CSS, making the HTML cleaner and easier to maintain.

8
v-forディレクティブのキーによりユニークな識別子を使用する。


v-forディレクティブを使用する際には、特にリストが大きくなる可能性がある場合、パフォーマンスの観点からv-bind:keyにユニークな値を設定することが重要です。team.idがユニークであることを確認してください。

src/components/User/ContestsContainer.vue [19-20]

 v-for="team in contest.teams"
-:key="team.id"
+:key="`team-${team.id}`"
 
Suggestion importance[1-10]: 7

Why: Ensuring the uniqueness of keys in v-for loops can prevent potential rendering issues and improve performance, especially with large lists.

7
Possible issue
テキストのインデントをマージンの調整に置き換える。

スタイル .detail に対して text-indent
の負の値を設定していますが、これは予期せぬレイアウトの問題を引き起こす可能性があります。代わりにマージンやパディングを調整することを検討してください。

src/components/User/ContestsContainer.vue [66]

-text-indent: -1rem;
+margin-left: -1rem;
 
Suggestion importance[1-10]: 6

Why: Using negative text-indent can cause unexpected layout issues; adjusting margins is a more predictable and maintainable approach.

6
Enhancement
スタイルの整合性を保つためにパディングを調整する。

新しいスタイルルール .detailpadding-left
を追加していますが、他の関連するスタイルとの整合性を保つために全体のデザインを見直すことをお勧めします。特に、他の要素との間隔が均一であるか確認してください。

src/components/User/ContestsContainer.vue [64]

-padding-left: 1.75rem;
+padding-left: 2rem;  /* 他の要素との整合性を保つために調整 */
 
Suggestion importance[1-10]: 5

Why: While adjusting padding for consistency is beneficial, the suggested value change to 2rem should be verified against the overall design to ensure it aligns with other elements.

5

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.

確認しました!よさそうです!
遅くなってごめん...

@Pugma Pugma merged commit 83e1cc8 into master Jul 6, 2024
9 checks passed
@Pugma Pugma deleted the feat/multipleContestTeam branch July 6, 2024 15:34
@mehm8128
Copy link
Member

mehm8128 commented Jul 6, 2024

すみませんマージ後ですけど一応
AIのレビューの1つ目は対応した方がよさそうなのと、3つ目のも特別text-indent使う理由があんまりなさそうなのでpaddingとかmarginとかを、ずらしたい要素につけた方が分かりやすそうかなって思いました(paddingとかmarginでもマイナスでつけるのはあんまりよくないのでそこはいい感じにしたいけど)

@mehm8128
Copy link
Member

mehm8128 commented Jul 6, 2024

(デザイン変えるところだと思うのでそんなに気にしなくて大丈夫です)

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.

コンテストの所属チームが1つしか表示されない
3 participants