Feat/modal #54

tesso57 · 2023-01-03T18:36:03Z

close ConfirmModalの実装 #28
下のように、Index.vueを変更すると動作確認ができます。
Dialogのオープンとクローズをコンポーネントに持たせたところが悩んでます。
- いい感じに分離させたいかも

<script lang="ts" setup>
import ContentHeader from '/@/components/Layout/ContentHeader.vue'
import PageInfoPanels from '/@/components/Index/PageInfoPanels.vue'
import PageContainer from '/@/components/Layout/PageContainer.vue'
import ConfirmModal from '../components/UI/ConfirmModal.vue'
import useModal from '../components/UI/composables/useModal'

const { modalRef, open, close } = useModal()
</script>

<template>
  <button @click="open">HELLO</button>
  <confirm-modal
    ref="modalRef"
    title="コンテストの削除"
    body="コンテンストと、コンテストに含まれるチームをすべて削除します。この操作は取り消せません。"
    @close="close"
  />
  <page-container>
    <content-header
      icon-name="mdi:apps"
      :header-texts="[{ title: 'Top', url: '/' }]"
      detail="ポートフォリオの設定を変更します"
      :class="$style.header"
    />
    <page-info-panels />
  </page-container>
</template>

<style lang="scss" module>
.header {
  margin: 4rem 0 2rem;
}
</style>

mehm8128

いくつか気になったところ書きました🙏

mehm8128 · 2023-01-04T04:06:11Z

src/components/UI/ConfirmDialog.vue

+        <base-button
+          type="secondary"
+          icon="mdi:arrow-left"
+          @click="() => emit('close')"


Reactじゃないのでこれでよさそうです

Suggested change

@click="() => emit('close')"

@click="emit('close')"

mehm8128 · 2023-01-04T04:07:08Z

src/components/UI/ConfirmDialog.vue

+}
+
+.container {
+  margin: 1rem 1.5rem;


paddingにしないとモーダルの中でもタイトルの少し上をクリックすると閉じちゃいそうです

Suggested change

margin: 1rem 1.5rem;

padding: 1rem 1.5rem;

左右はFigmaの通り1remにしたら2行に収まりそうです

mehm8128 · 2023-01-04T04:07:20Z

src/pages/Index.vue

+  <confirm-modal />
+


消し忘れですかね

mehm8128 · 2023-01-04T04:08:15Z

src/pages/Index.vue

@@ -5,6 +5,8 @@ import PageContainer from '/@/components/Layout/PageContainer.vue'
 </script>

 <template>
+  <confirm-modal />


例示していたここらへんをcomposablesとしてまとめるのがよさそうです？

const confirmDialogRef = ref() const open = () => { confirmDialogRef.value?.open() } const close = () => { confirmDialogRef.value?.close() }

mehm8128 · 2023-01-04T04:09:57Z

src/components/UI/ConfirmDialog.vue

@@ -0,0 +1,91 @@
+<script lang="ts" setup>


コンポーネント名、ConfirmDialogじゃなくてConfirmModalの方がよさそうです(ちょっと調べてみた感じ、多分Dialogだと8行目でshowModal()じゃなくてshow()にしたときみたいな挙動になりそうです)

mehm8128 · 2023-01-04T04:15:27Z

src/components/UI/ConfirmDialog.vue

+const dialogRef = ref<HTMLDialogElement>()
+
+const open = () => {
+  dialogRef.value?.showModal()


オプショナルチェーン使うか18行目みたいに早期returnするかで統一したい気がするんですけど何か違いってありますか？

mehm8128 · 2023-01-04T04:17:35Z

src/components/UI/ConfirmDialog.vue

+      <h1 :class="$style.header">コンテストの削除</h1>
+      <p :class="$style.body">
+        コンテンストと、コンテストに含まれるチームをすべて削除します。この操作は取り消せません。
+      </p>
+      <div :class="$style.buttonContent">
+        <base-button
+          type="secondary"
+          icon="mdi:arrow-left"
+          @click="() => emit('close')"
+          >Back</base-button
+        >
+        <base-button
+          type="warning"
+          icon="mdi:close"
+          @click="() => emit('remove')"
+          >Remove</base-button
+        >


コンテスト以外でも使うと思うので、いい感じにpropsを渡すようにしたいです
削除にしかこのコンポーネントを使わないかどうかでpropsの種類が変わってきそう

mehm8128 · 2023-01-04T04:19:23Z

src/components/UI/ConfirmDialog.vue

+          type="warning"
+          icon="mdi:close"
+          @click="() => emit('remove')"
+          >Remove</base-button


多分この場合、removeじゃなくてdeleteの方が適切な気がします(チームからメンバーを削除とかならメンバー自体は存在しているのでremoveだと思いますが、コンテストは完全に消去してしまうのでdeleteだと思います)

mehm8128

よさそうです！提案レベルなのですが1点だけ追加で🙏

mehm8128 · 2023-03-22T11:33:04Z

src/components/UI/ConfirmModal.vue

+        <base-button
+          type="secondary"
+          icon="mdi:arrow-left"
+          @click="emit('close')"
+          >Back</base-button
+        >


今更で申し訳ないんですけど、モーダルにBackってボタンあるのちょっと違和感ある気がします(実際には閉じるだけだけどページごと戻っちゃう気もしちゃいそう？)
Cancelにしてアイコンちょっと変えたりするといいかもと思いましたがどうでしょう👀

たしかに

mehm8128

適当なアイコン見つけるの難しそうであれば変更前の状態に戻しておいてもらえればそれで一旦approveします

mehm8128 · 2023-05-22T13:12:13Z

src/components/UI/ConfirmModal.vue

        >
        <base-button type="warning" icon="mdi:close" @click="emit('remove')"
-          >Delete</base-button
+          >Cancel</base-button


ここはDeleteのままだと思います

mehm8128 · 2023-05-22T13:12:46Z

src/components/UI/ConfirmModal.vue

@@ -52,10 +52,10 @@ const emit = defineEmits<{
          type="secondary"
          icon="mdi:arrow-left"
          @click="emit('close')"
-          >Back</base-button
+          >Close</base-button
        >
        <base-button type="warning" icon="mdi:close" @click="emit('remove')"


remove=>deleteしたときにemitの値も変え忘れてそうです

mehm8128 · 2023-05-22T13:13:50Z

src/components/UI/ConfirmModal.vue

@@ -52,10 +52,10 @@ const emit = defineEmits<{
          type="secondary"
          icon="mdi:arrow-left"
          @click="emit('close')"
-          >Back</base-button
+          >Close</base-button


ここなるボタンでCloseってあんまり見ない気がします、ここを「Cancel」にするという意図でした(アイコンも変えてほしいです)

tesso57 · 2023-05-23T12:14:18Z

コミットメッセージとやっていることが全然違ってしまって申し訳ないです。
ボタン周りの文言を修正しました。

元々、Delete のアイコンは✗マークだったんですが、Cancel とアイコンがかぶるので、ゴミ箱マークにしました。
Figmaの方も変えてあります。
https://www.figma.com/file/BVnhB2JyTjKsGKdnAJDksb/traPortfolio?type=design&node-id=2241-1418&t=eAw5yRiz5TU0JjQi-4

mehm8128

よさそうです！

tesso57 added 5 commits January 3, 2023 23:14

🚧 原型の完成

52a06c8

✨ impl modal

c319e66

✨ impl dialog

d48af7e

🔥 change name

8bb8f63

🐛 不要なimportを削除

999484c

mehm8128 reviewed Jan 4, 2023

View reviewed changes

✏️ 消し忘れ

f251119

mehm8128 mentioned this pull request Jan 8, 2023

/contest/:contestId/teams/:teamId/edit #10

Closed

tesso57 added 2 commits March 8, 2023 20:57

⚡️ useModalを利用して、modalのロジックを分離

e7748a0

♻️ 名前を変更, 見た目を統一

872a27d

tesso57 self-assigned this Mar 8, 2023

tesso57 requested a review from mehm8128 March 22, 2023 11:00

mehm8128 reviewed Mar 22, 2023

View reviewed changes

toshi-pono mentioned this pull request Apr 24, 2023

コンテストチーム削除の実装 #69

Closed

🐛 Back から Cancel に変更

ed5e9e1

mehm8128 reviewed May 22, 2023

View reviewed changes

tesso57 added 2 commits May 23, 2023 21:06

🐛 icon の変更

510c5ac

🐛 Remove => Delete

726dfb7

tesso57 requested a review from mehm8128 May 23, 2023 12:14

mehm8128 approved these changes May 25, 2023

View reviewed changes

tesso57 merged commit b18cf56 into main May 26, 2023

tesso57 deleted the feat/modal branch May 26, 2023 08:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/modal #54

Feat/modal #54

tesso57 commented Jan 3, 2023 •

edited

Loading

mehm8128 left a comment

mehm8128 Jan 4, 2023

mehm8128 Jan 4, 2023

mehm8128 Jan 4, 2023

mehm8128 Jan 4, 2023

mehm8128 Jan 4, 2023

mehm8128 Jan 4, 2023

mehm8128 Jan 4, 2023

mehm8128 Jan 4, 2023

mehm8128 Jan 4, 2023

mehm8128 left a comment

mehm8128 Mar 22, 2023

tesso57 May 22, 2023

mehm8128 left a comment

mehm8128 May 22, 2023

mehm8128 May 22, 2023

mehm8128 May 22, 2023

tesso57 commented May 23, 2023

mehm8128 left a comment

Feat/modal #54

Feat/modal #54

Conversation

tesso57 commented Jan 3, 2023 • edited Loading

mehm8128 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mehm8128 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mehm8128 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tesso57 commented May 23, 2023

mehm8128 left a comment

Choose a reason for hiding this comment

tesso57 commented Jan 3, 2023 •

edited

Loading