Skip to content

Conversation

@mumumu6
Copy link
Member

@mumumu6 mumumu6 commented Dec 11, 2025

menuがbody部分に描画されるらのでglobalにcssを当てています。
タイトル部分はdeepで反映させています

Copilot AI review requested due to automatic review settings December 11, 2025 13:41
@github-actions
Copy link

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enables text wrapping for survey question titles and options in the QuestionEditField component to handle long text content. The implementation adds CSS styling to allow multi-line text display in select dropdowns and input fields, with mock data updated to demonstrate the functionality with longer text examples.

Key Changes

  • Added CSS rules to enable text wrapping for select dropdowns and their options by setting white-space: normal and overflow: visible
  • Applied a multi-line-select class to all form input elements (text fields, number inputs, and select dropdowns)
  • Updated mock data with longer text examples to test the wrapping behavior

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
src/mocks/handlers/questions.ts Updated mock data with longer title and option text to demonstrate text wrapping functionality
src/components/information/QuestionEditField.vue Added multi-line-select class to all form elements and CSS rules to enable text wrapping for labels, selections, and dropdown options

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

:rules="[requiredRule]"
:disabled="!question.isOpen"
variant="underlined"
class="multi-line-select"
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

The class name "multi-line-select" is misleading when applied to a v-text-field component. This is a text input field, not a select element. Consider using a more accurate name like "multi-line-input" or "wrap-text" that better describes its purpose of allowing text wrapping.

Copilot uses AI. Check for mistakes.
:disabled="!question.isOpen"
variant="underlined"
control-variant="hidden"
class="multi-line-select"
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

The class name "multi-line-select" is misleading when applied to a v-number-input component. This is a number input field, not a select element. Consider using a more accurate name like "multi-line-input" or "wrap-text" that better describes its purpose of allowing text wrapping.

Copilot uses AI. Check for mistakes.
Comment on lines 72 to 76
:global(.v-list-item-title) {
white-space: normal;
overflow: visible;
text-overflow: clip;
}
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

Using :global(.v-list-item-title) without a parent scoping class will affect all v-list-item-title elements across the entire application, which may cause unintended side effects in other components. Following the pattern used in other components like EventEditorSettings.vue and QuestionShowField.vue, this should be scoped to a parent class, such as .multi-line-select :global(.v-list-item-title) to limit its impact to only the dropdown lists within this component.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

@kitsne241 kitsne241 Dec 11, 2025

Choose a reason for hiding this comment

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

Copilot による Review でも触れられていますが、CSS セレクタで :global とか :deep を使う場合は頭にクラスをつけると影響範囲が限定されて安心できそうです

Comment on lines 65 to 77
<style scoped>
.multi-line-select :deep(.v-select__selection-text) {
white-space: normal;
overflow: visible;
text-overflow: clip;
}

:global(.v-list-item-title) {
white-space: normal;
overflow: visible;
text-overflow: clip;
}
</style>
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

The component uses <style scoped> but other similar components in the codebase (EventEditorSettings.vue, QuestionShowField.vue) use <style module> with $style references in the template. Consider following the existing pattern by using CSS modules for consistency with the codebase conventions.

Copilot uses AI. Check for mistakes.
@mumumu6 mumumu6 requested a review from kitsne241 December 11, 2025 13:51
Copy link
Collaborator

@kitsne241 kitsne241 left a comment

Choose a reason for hiding this comment

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

問題なく動くことは確認しました 👍
細かい部分に変更依頼をつけたので見てもらえると嬉しいです!

:rules="[requiredRule]"
:disabled="!question.isOpen"
variant="underlined"
class="wrap-text"
Copy link
Collaborator

Choose a reason for hiding this comment

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

<style module> 式に

Suggested change
class="wrap-text"
:class="$style.wrapText"

と書いて欲しいかも。他の部分をふくむ

Comment on lines 65 to 66
<style scoped>
.wrap-text :deep(.v-select__selection-text) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<style scoped>
.wrap-text :deep(.v-select__selection-text) {
<style module>
.wrapText :deep(.v-select__selection-text) {

CSS Modules への変更をお願いしたいです!

Copy link
Member Author

Choose a reason for hiding this comment

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

moduleにするんだった。忘れてた。ありがとう!!

Comment on lines 72 to 76
:global(.v-list-item-title) {
white-space: normal;
overflow: visible;
text-overflow: clip;
}
Copy link
Collaborator

@kitsne241 kitsne241 Dec 11, 2025

Choose a reason for hiding this comment

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

Copilot による Review でも触れられていますが、CSS セレクタで :global とか :deep を使う場合は頭にクラスをつけると影響範囲が限定されて安心できそうです

@mumumu6 mumumu6 requested a review from kitsne241 December 12, 2025 03:09
Copy link
Collaborator

@kitsne241 kitsne241 left a comment

Choose a reason for hiding this comment

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

もう一点だけ確認をお願いしたい部分があります! お手数おかけしてすみません

white-space: normal !important;
}

:global(.v-overlay-container) :global(.v-list-item-title) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
:global(.v-overlay-container) :global(.v-list-item-title) {
.wrapText :global(.v-list-item-title) {

という限定をすると影響をこのコンポーネント(QuestionEditField.vue)の中に閉じ込めることができてうれしそう

CSS Modules によって、このスタイルの .wrapText の部分がプロダクト全体で一意な文字列に置き換えられて dist に含まれることになります

Copy link
Member Author

Choose a reason for hiding this comment

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

v-list-item-title(プルダウンででてくるやつ)なんですがvuetifyの仕様でteleportされてbody直下のv-overlay-containerに飛ばされるんですよね。
試してみたんですがquestion editerfieldの外で描画されていて上手くいかなかったです。

このコンポーネントとは別の箇所にcssで影響を与えるため先頭を:globalにしています。

Copy link
Member Author

Choose a reason for hiding this comment

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

せめてもの制限でoverlay-containerを先頭に付けていますが別のコンポーネントでも同じようなv-selectを使うと同様に折り返しが強制されてしまいそうです。
避けるの難しいかも

@mumumu6 mumumu6 requested a review from kitsne241 December 14, 2025 04:41
@mumumu6
Copy link
Member Author

mumumu6 commented Dec 14, 2025

templateを使うことでglobalに反映させることなく部分的に折り返しを反映できたので修正しました。

Copy link
Collaborator

@kitsne241 kitsne241 left a comment

Choose a reason for hiding this comment

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

LGTMです!
これは前回のレビューで言及してなくて申し訳ないのですが、QuestionShowField.vue の方にも同様の変更を入れてもらえるととても助かります

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