questionnaire/newのText系の実装 #920

wakashi1 · 2022-06-18T07:02:42Z

No description provided.

modelvalue追加しないとCheckboxElementの入れ替えでうまくいかないこと忘れてました。

InputTextで代用できるのでいらないかもしれません

wakashi1 · 2022-06-18T07:09:18Z

レビューお願いします

ryoha000

いい感じだと思います:+1:

src/components/NewQuestionnaire/Questions.vue

ryoha000 · 2022-06-18T11:31:08Z

src/components/UI/ToggleSwitch.vue

@@ -0,0 +1,70 @@
+<template>


これをUIいかに置いてくれてるのはいいですね:+1:

src/components/NewQuestionnaire/Forms/QuestionCopy.vue

src/components/NewQuestionnaire/Forms/RequiredSwitch.vue

src/components/UI/Select.vue

wakashi1 · 2022-06-19T14:59:39Z

だいたいは直したんですけど、直してない点が二つくらいあります。
まず一つ目はUI/Select.vueのpointerevents:noneで、これがないと下やじるしみたいなやつの上でSelectの部分が選択できなくなるので残したままにしときました。
二つ目は、Questions.vueの切り出しでQuestionUpdownの部分をQuestion.vueに切り出しませんでした。これはQuestionupdownだけがiに依存していて他は依存していなかったので、Questionupdownとそれ以外で分けたほうがいいかな～と思って切り出しませんでした。
それ以外は直ってると思います。

ryoha000

通話で言ったこと

ryoha000 · 2022-06-21T12:38:38Z

src/components/NewQuestionnaire/Questions.vue

+            <QuestionUpdown
+              :index="i"
+              :max="questions.length"
+              :class="$style.border"
+              @swap="swapQuestions"
+            />


Suggested change

<QuestionUpdown

:index="i"

:max="questions.length"

:class="$style.border"

@swap="swapQuestions"

/>

<QuestionUpdown

:class="$style.border"

:up-disable="i === 0"

:down-disable="i === questions.length - 1"

@up="() => swapQuestions(i, i - 1)"

@down="() => swapQuestions(i, i + 1)"

/>

こんな感じにしてほしい

ryoha000 · 2022-06-21T12:39:10Z

src/components/NewQuestionnaire/Question.vue

@@ -0,0 +1,125 @@
+<template>


QuestionContentにしてほしい

ryoha000 · 2022-06-21T12:42:46Z

src/components/NewQuestionnaire/Forms/QuestionForm.vue

-        return false
-      }
-    })
+    const questionIsRequired = computed(() => props.isRequired)


これ自体なくてよくない？
updateQuestionIsRequiredはprops使えばいい気がする

wakashi1 · 2022-06-24T23:58:09Z

figmaをみて直してみました。前のレビューで言われたところでfigmaと違うところがあったので、figmaのほうに合わせたんですが直さないほうがよかったらすみません

ryoha000 · 2022-06-25T16:48:45Z

src/components/NewQuestionnaire/QuestionContent.vue

+import QuestionTypeSelect from './Forms/QuestionTypeSelect.vue'
+
+export default defineComponent({
+  name: 'Question',


Suggested change

name: 'Question',

name: 'QuestionContent',

ryoha000 · 2022-06-25T16:59:21Z

src/components/NewQuestionnaire/Forms/QuestionCopyButton.vue

+    index: {
+      type: Number,
+      required: true
+    }


これいらないよね？

ryoha000 · 2022-06-25T17:00:17Z

src/components/NewQuestionnaire/Forms/QuestionDeleteButton.vue

-    questionarray: {
-      type: Array,
-      required: true
-    },
    index: {


おなじくいらない？

ryoha000 · 2022-06-25T17:31:30Z

src/components/NewQuestionnaire/Forms/RequiredSwitch.vue

+  width: 28px;
+  height: 21px;


これは指定しなくていいかな(意図として指定したいのはfontsizeとline-heightなので)

ryoha000 · 2022-06-25T17:57:05Z

src/components/UI/InputText.vue

    color: $ui-secondary;
  }
+  &:hover {
+    background-color: $bg-secondary-highlight;


アクションに対して色変えるときはtransitionほしいです

ryoha000 · 2022-06-25T19:04:16Z

src/components/UI/InputText.vue

@@ -1,8 +1,13 @@
 <template>
-  <div>
+  <div :class="[isNumber ? $style.spin : '']">


複雑になっちゃったんでInputTextBaseみたいなのをつくってNumberとTextでわけたいですね

ryoha000 · 2022-06-25T19:06:25Z

src/components/UI/InputText.vue

+    nonEvents: {
+      type: Boolean,
+      default: false
+    },


pointer-eventsがnoneかどうかっていうprops名じゃなくてどういう場合にこのpropsを渡すのかっていうprops名にしたいです

src/components/UI/ToggleSwitch.vue

ryoha000 · 2022-06-25T19:16:23Z

src/components/UI/ToggleSwitch.vue

-  background-color: $ui-primary;
+  width: 34px;
+  height: 20px;
+  border-radius: 9999px;


意図が分かりづらいんでheight/2で指定してほしいです

ryoha000 · 2022-06-25T19:16:36Z

src/components/UI/ToggleSwitch.vue

  border-radius: 50%;
  background-color: $ui-white;
+  opacity: 1;


いらないかな？

wakashi1 · 2022-06-29T06:29:03Z

figmaみたら新規作成のところでInputTextいらなかったぽかったので、直しました。

wakashi1 · 2022-07-01T11:49:31Z

InputNumberとInputTextBaseを消しました

ryoha000 · 2022-07-01T11:37:09Z

src/components/UI/InputNumber.vue

+    nonEvents: {
+      type: Boolean,
+      default: false
+    },


ここ変えてほしいです

ryoha000 · 2022-07-01T11:42:00Z

src/components/UI/InputNumber.vue

+    modelValue: {
+      type: String,
+      required: true
+    }


numberで受け取りたいです

ryoha000 · 2022-07-01T11:42:53Z

src/components/UI/InputNumber.vue

@@ -0,0 +1,139 @@
+<template>
+  <div :class="$style.spin">


Suggested change

<div :class="$style.spin">

<div :class="$style.container">

かな

ryoha000 · 2022-07-01T11:43:24Z

src/components/UI/InputText.vue

+    isLong: {
+      type: Boolean,
+      default: false
+    },


いらない？

ryoha000 · 2022-07-01T11:47:41Z

src/components/UI/InputNumber.vue

+.input {
+  padding: 4px 8px;
+  width: 100%;
+  height: 32px;
+  @include size-body;
+  color: $ui-primary;
+  box-sizing: border-box;
+  border: none;
+  border-bottom: $input-border solid $ui-secondary;
+  outline: none;
+  &::placeholder {
+    padding: 4px 8px;
+    width: 100%;
+    height: 24px;
+    @include size-body-small;
+    color: $ui-secondary;
+  }
+  &:hover {
+    background-color: $bg-secondary-highlight;
+    transition: 0.1s;
+  }
+  &:focus {
+    background-color: $bg-secondary-highlight;
+  }
+}


ここが多分共通だと思うんでInputBaseでうまいこと共通化したいです

ryoha000 · 2022-07-01T11:49:25Z

src/components/UI/InputTextBase.vue

+import InputText from './InputText.vue'
+
+export default defineComponent({
+  name: 'InputTextBase',


Suggested change

name: 'InputTextBase',

name: 'InputBase',

ryoha000 · 2022-07-01T11:53:34Z

src/components/UI/InputTextBase.vue

@@ -0,0 +1,60 @@
+<template>


<tempate> <input 共通のクラスを付ける /> </template>

で、InputBaseWithUnderlineで

<tempate> <div> <slot /> <underline /> </div> </template>

で、InputTextとかで

<tempate> <input-base-with-underline> <input-base Text固有のクラスを付ける /> </input-base-with-underline> </template>

ですかね？
こっちのほうがスマートじゃね？って思ったら言ってください

ryoha000 · 2022-07-01T11:54:49Z

コミットされる前のやつを見てコメントしちゃった
まぁ回答画面作るときの参考にしてください

ryoha000

多分いいと思う:+1:

wakashi1 added 13 commits June 15, 2022 22:57

create copybutton

d14aa54

fix InputText.vue

358a154

modelvalue追加しないとCheckboxElementの入れ替えでうまくいかないこと忘れてました。

create UI/ToggleSwitch.vue

3d3d66c

fix Icon

6dff8de

fix css in UI/Select.vue

b8e103e

fix InputText.vue

5cf43d1

fix QuestionTitle.vue

01c6d6e

InputTextで代用できるのでいらないかもしれません

add findQuestionTypes

cffffd8

fix css and Questioncopy in Question.vue

3466c23

fix TextForm.vue

0cddc18

create RequiredSwitch.vue

c87e10a

fix css and requiredswitch in QuestionForm.vue

b95e98a

create QuestionTypeSelect.vue

ec50c7c

ryoha000 mentioned this pull request Jun 18, 2022

新規作成画面で質問の種類を変えるときに変更前のものから一部情報を引き継ぎたい #921

Closed

ryoha000 reviewed Jun 18, 2022

View reviewed changes

wakashi1 added 11 commits June 19, 2022 23:45

rm trashcan in mdi.ts

e821a6a

QuestionCopy to QuestionCopyButton

c446a18

QuestionDispose.vue to QuestionDeleteButton.vue

60c2cb4

fix QuestionForm.vue

12707ce

fix RequiredSwitch.vue

263b9db

create Question.vue

a992d71

fix Questions.vue

131249d

fix util.ts

f50a5b4

rm UI/QuestionTitle.vue

13bef11

fix css in UI/Select.vue

b8971b5

fix UI/ToggleSwitch.vue

3b6f637

ryoha000 reviewed Jun 21, 2022

View reviewed changes

wakashi1 added 7 commits June 24, 2022 21:15

use computed in QuestionContent.vue

93087f2

fix TextForm.vue

2362dd0

fix css in RequiredSwitch.vue

54158da

fix css in Questions.vue

f1dfdfc

fix css in InputText.vue

759b339

fix css in ToggleSwitch.vue

86716a9

fix RequiredSwitch.vue

7a13e9e

ryoha000 reviewed Jun 25, 2022

View reviewed changes

wakashi1 added 12 commits June 26, 2022 11:11

sizeの登録

8e73ee9

fix css in ToggleSwitch.vue

204b709

rm props in QuestionCopyButton and QuestionDeleteButton

185343a

fix css in RequiredSwitch.vue

dacdd07

fix name in QuestionContent.vue

bb120a7

fix css in InputText.vue

e007712

Merge branch 'master' into new-question-Implementation

a95f9f5

fix css in RequiredSwich.vue

5366227

fix TextForm.vue

da2c209

create InputNumber.vue

02ca153

fix InputText.vue

a752710

create InputTextBase.vue

744f5a9

rm InputNumber.vue and InputTextBase.vue

ffb936f

ryoha000 reviewed Jul 1, 2022

View reviewed changes

ryoha000 approved these changes Jul 1, 2022

View reviewed changes

ryoha000 merged commit 2893465 into master Jul 1, 2022

ryoha000 deleted the new-question-Implementation branch July 1, 2022 11:58

ryoha000 mentioned this pull request Aug 20, 2022

Responses/new/idの作成 #980

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

questionnaire/newのText系の実装 #920

questionnaire/newのText系の実装 #920

wakashi1 commented Jun 18, 2022

wakashi1 commented Jun 18, 2022

ryoha000 left a comment

ryoha000 Jun 18, 2022

wakashi1 commented Jun 19, 2022

ryoha000 left a comment

ryoha000 Jun 21, 2022

ryoha000 Jun 21, 2022

ryoha000 Jun 21, 2022

wakashi1 commented Jun 24, 2022

ryoha000 Jun 25, 2022

ryoha000 Jun 25, 2022

ryoha000 Jun 25, 2022

ryoha000 Jun 25, 2022

ryoha000 Jun 25, 2022

ryoha000 Jun 25, 2022

ryoha000 Jun 25, 2022

ryoha000 Jun 25, 2022

ryoha000 Jun 25, 2022

wakashi1 commented Jun 29, 2022

wakashi1 commented Jul 1, 2022

ryoha000 Jul 1, 2022

ryoha000 Jul 1, 2022

ryoha000 Jul 1, 2022

ryoha000 Jul 1, 2022

ryoha000 Jul 1, 2022

ryoha000 Jul 1, 2022

ryoha000 Jul 1, 2022

ryoha000 commented Jul 1, 2022

ryoha000 left a comment

questionnaire/newのText系の実装 #920

questionnaire/newのText系の実装 #920

Conversation

wakashi1 commented Jun 18, 2022

wakashi1 commented Jun 18, 2022

ryoha000 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wakashi1 commented Jun 19, 2022

ryoha000 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

wakashi1 commented Jun 24, 2022

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

wakashi1 commented Jun 29, 2022

wakashi1 commented Jul 1, 2022

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

ryoha000 commented Jul 1, 2022

ryoha000 left a comment

Choose a reason for hiding this comment