-
Notifications
You must be signed in to change notification settings - Fork 40
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
FormSelectorFilterable.vueを作成 #4244
Conversation
Preview (prod) → https://4244-prod.traq-preview.trapti.tech/ |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4244 +/- ##
=======================================
Coverage 86.36% 86.36%
=======================================
Files 66 66
Lines 4722 4722
Branches 565 565
=======================================
Hits 4078 4078
Misses 638 638
Partials 6 6 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#2261 の対応ってことで合ってますか?
合ってたらissue紐づけておいてほしいです
type Optionを作成 検索結果0のときの表示を作成 cssを仮作成
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get startedって読みましたか?
今回はexisting projectなので多分ここらへんを読むことになります
https://vuetifyjs.com/en/getting-started/installation/#existing-projects
が、vuetifyって基本的に他のコンポーネントもこれに依存させて使うことが前提っぽいので、今回使うには向いてない気がします(vue詳しくないので分からないですが)
This reverts commit 15a028c.
FormSelectorFilterable.vueを作成
Todo
|
いろいろ考えたんですけどあんまり体験がよくならなかったのでこのままでいきます。アーカイブチャンネルを削って軽くなることを期待したいですが、それでも重ければ |
僕としてはこれで一旦完成としてマージしちゃいたいので、よければapproveお願いします🙇♂️ |
あれ、前にレビューしたところいくつか見逃してそうなので確認してほしいです👀 |
今新しく1つつけたのと、↓をお願いします |
あと修正終わったら毎回review requestを投げ直してほしいです(修正が続いてるのか完了してるのか分からないので) |
ごめんなさい!これ知らなかったです、ご迷惑をおかけしました |
@include background-secondary; | ||
&[background='primary'] { | ||
@include background-primary; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
デフォルトがsecondaryで条件分岐でprimaryを当ててるのちょっと不自然なので、逆にした方がいいかもです
他のところも同様
@include background-primary;
&[background='secondary'] {
@include background-secondary;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FormSelector.vueもそうなんですけど、primaryの上に置くことが多いのでデフォがsecondaryになってるんだと思います。なのでデフォはsecondaryにするのが自然かなーって思ってるんですけど、どうしましょう?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
確かにそうかもです
このままで大丈夫そうです👍
多分見えてると思います! #4244 (comment) ですよね、そっちのほうで返信してるのでなんか変なこと言ってたら教えてください |
あれ、逆にその返信が見えてないかもです:thonk_sweat: |
返信したつもりのコメントに「Pending」バッジがついてたら返信できてないので、記事に従って送信してみてほしいです |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
できました!できてますか? |
見えるようになりました👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
よさそうです、ありがとうございました!
了解です!長々と付き合ってくださってありがとうございました |
No description provided.