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

申請作成ページを Figma に合わせる #167

Merged
merged 13 commits into from
Jul 22, 2024
Merged

Conversation

anko9801
Copy link
Contributor

No description provided.

@anko9801 anko9801 self-assigned this May 18, 2024
@anko9801 anko9801 requested a review from reiroop May 18, 2024 04:25
@anko9801 anko9801 linked an issue May 18, 2024 that may be closed by this pull request
@anko9801 anko9801 requested a review from mehm8128 May 18, 2024 04:30
src/pages/NewRequestPage.vue Outdated Show resolved Hide resolved
src/pages/NewRequestPage.vue Outdated Show resolved Hide resolved
src/components/shared/MarkdownTextarea.vue Outdated Show resolved Hide resolved
src/components/newRequest/NewRequestTargets.vue Outdated Show resolved Hide resolved
src/pages/NewRequestPage.vue Outdated Show resolved Hide resolved
@anko9801 anko9801 requested a review from mehm8128 May 18, 2024 14:17
@mehm8128
Copy link
Contributor

mehm8128 commented Jul 5, 2024

/improve

Copy link

github-actions bot commented Jul 5, 2024

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Score
Possible bug
props.targets の長さが 1 以上であるかのチェックを追加して、全てのターゲットが削除されるのを防ぐ。

props.targets.filter メソッドを使用しているが、削除する前に props.targets の長さが 1
以上であるかのチェックが必要です。これは、全てのターゲットを削除してしまうことを防ぐためです。

src/components/newRequest/NewRequestTargets.vue [52]

-emit(
-  'input',
-  props.targets.filter((_, i) => i !== index)
+if (props.targets.length > 1) {
+  emit(
+    'input',
+    props.targets.filter((_, i) => i !== index)
+  )
+} else {
+  // エラーメッセージを表示する処理を追加
+}
 
Suggestion importance[1-10]: 9

Why: This suggestion addresses a potential bug where all targets could be removed if there is only one target left. Adding a check to ensure at least one target remains is crucial for preventing unintended data loss.

9
Accessibility
タグに aria-label 属性を追加して、アクセシビリティを向上させる。

<button> タグ内で TrashIcon を使用していますが、アクセシビリティを向上させるために aria-label 属性を追加することを推奨します。

src/components/newRequest/NewRequestTargets.vue [80-85]

 <button
   v-if="targets.length > 1"
   class="flex"
+  aria-label="ターゲットを削除"
   @click="handleRemoveTarget(i)">
   <TrashIcon class="w-6 text-red-400" />
 </button>
 
Suggestion importance[1-10]: 8

Why: Adding an aria-label attribute to the <button> tag enhances accessibility, making the application more usable for people with disabilities. This is an important improvement for inclusivity.

8
Enhancement
タグに name 属性を追加して、フォームデータの識別を容易にする。

<input> タグに name 属性が設定されていません。フォームデータを送信する際に、サーバー側で識別しやすくするために name 属性を追加することをお勧めします。

src/components/newRequest/NewRequestFileForm.vue [49-54]

 <input
   id="image"
+  name="image"
   ref="inputRef"
   multiple
   type="file"
   @change="handleFileChange" />
 
Suggestion importance[1-10]: 7

Why: Adding a name attribute to the <input> tag improves form data handling on the server side. This is a good enhancement for better data management.

7
accept 属性を追加して、アップロードできるファイルの種類を制限する。

accept
属性を追加して、アップロードできるファイルの種類を制限することをお勧めします。これにより、ユーザーがサポートされていないファイル形式を誤ってアップロードするのを防ぐことができます。

src/components/newRequest/NewRequestFileForm.vue [49-54]

 <input
   id="image"
   ref="inputRef"
   multiple
   type="file"
+  accept="image/png, image/jpeg"
   @change="handleFileChange" />
 
Suggestion importance[1-10]: 7

Why: Adding an accept attribute to the <input type="file"> tag restricts the types of files that can be uploaded, preventing user errors and ensuring only supported file formats are uploaded. This is a useful enhancement.

7

@anko9801 anko9801 requested a review from mehm8128 July 15, 2024 01:50
@anko9801 anko9801 requested a review from mehm8128 July 22, 2024 03:02
Copy link
Contributor

@mehm8128 mehm8128 left a comment

Choose a reason for hiding this comment

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

一旦OKだと思います

@anko9801 anko9801 merged commit 3a4408a into main Jul 22, 2024
6 checks passed
@anko9801 anko9801 deleted the feat/new_request branch July 22, 2024 03:08
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