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

ServiceAccordion の実装 #53

Merged
merged 10 commits into from
Apr 12, 2023
Merged

ServiceAccordion の実装 #53

merged 10 commits into from
Apr 12, 2023

Conversation

toshi-pono
Copy link
Member

close #29

ServiceAccordion を実装しました

確認方法

どのページにも実装されていないので、適当に表示してください:pray:

import { ref } from 'vue'
import ServiceAccordion from '/@/components/UI/ServiceAccordion.vue'
const value = ref(0)

~~~

<service-accordion v-model="value" />

@toshi-pono toshi-pono self-assigned this Jan 2, 2023
Comment on lines 47 to 57
.item {
display: grid;
grid-template-columns: 24px 1fr;
gap: 8px;
.icon {
color: $color-primary;
}
.label {
grid-column: 2;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

align-items: center;ほしいです

Copy link
Member Author

@toshi-pono toshi-pono Jan 2, 2023

Choose a reason for hiding this comment

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

どの部分を指してますか?(Homepageとかの文字列が入ってくるlabelの部分ですか?)
勘違いしてました:pray: 解決しました

</v-select>
</template>

<style lang="scss" module>
Copy link
Contributor

Choose a reason for hiding this comment

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

選択したときの選択した感と、カーソル当てたときの検索できる感があんまりない気がするのでcursorを選択肢をホバーしたときにpointer、selectbox自体にカーソル当てたときにtextにするのがいい気がします(これデフォルトでなってた気がするんですけどどこかで消してますっけ)

Copy link
Member Author

Choose a reason for hiding this comment

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

良さそうなのでやっておきます○

(デフォルトで

[aria-controls] {
  cursor: pointer;
}

になってて、これがまずそう?)

</script>

<template>
<base-select v-model="value" :options="options" searchable />
Copy link
Contributor

Choose a reason for hiding this comment

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

optionsについてなんですけど、この前同じサービスに対して複数アカウントを登録できなくしたので、既に登録してあるサービスを除外したい感があります

Copy link
Member Author

Choose a reason for hiding this comment

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

登録済みのサービスはdisableな見た目にして選択できない感じですかね?

(もしくは、完全に表示しない)

Copy link
Contributor

Choose a reason for hiding this comment

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

完全に表示しないのを想定していましたが、いい感じのUIにできそうであればdisableな見た目にした方が分かりやすいかもです🤨

@toshi-pono
Copy link
Member Author

toshi-pono commented Jan 2, 2023

スクロールバーがあるせいで、角丸のボーダーが隠れちゃってて微妙な感じになってる
CSSでスクロールバーのデザインを変更するか、はみ出た部分をhiddenで隠したい

はみ出た部分をhiddenで隠すほうが嬉しそうだけど、DOM構造的に vue-select側を修正しなきゃいけなくて大変そう><

or ......vue-selectを使うのをやめる?

スクリーンショット 2023-01-03 0 40 50

@mehm8128
Copy link
Contributor

mehm8128 commented Jan 3, 2023

スクロールバーがあるせいで、角丸のボーダーが隠れちゃってて微妙な感じになってる CSSでスクロールバーのデザインを変更するか、はみ出た部分をhiddenで隠したい

はみ出た部分をhiddenで隠すほうが嬉しそうだけど、DOM構造的に vue-select側を修正しなきゃいけなくて大変そう><

or ......vue-selectを使うのをやめる?

スクリーンショット 2023-01-03 0 40 50

#46 でスクロールバーのCSS変更すると思うのでそのときにいい感じにすれば良さそうです?

@toshi-pono
Copy link
Member Author

とりあえず、登録済みのサービスを表示しないようにしました

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.

よさそうです!
1点だけ気になったところあったのでお好みで修正お願いします🙏


interface Props {
modelValue: AccountType
registered: AccountType[]
Copy link
Contributor

Choose a reason for hiding this comment

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

optionalにしてデフォルトで空配列にしてもよさそうです👀

Copy link
Member Author

Choose a reason for hiding this comment

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

遅くなりました:pray:デフォルトで空配列にしました

@toshi-pono toshi-pono merged commit 43449d0 into main Apr 12, 2023
@toshi-pono toshi-pono deleted the toshi00/service-accordion branch April 12, 2023 12:13
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.

ServiceAccordion の実装
2 participants