Conversation
|
Preview (prod) → https://154-prod.rucq-ui-preview.trapti.tech/ |
There was a problem hiding this comment.
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: normalandoverflow: visible - Applied a
multi-line-selectclass 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" |
There was a problem hiding this comment.
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.
| :disabled="!question.isOpen" | ||
| variant="underlined" | ||
| control-variant="hidden" | ||
| class="multi-line-select" |
There was a problem hiding this comment.
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.
| :global(.v-list-item-title) { | ||
| white-space: normal; | ||
| overflow: visible; | ||
| text-overflow: clip; | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Copilot による Review でも触れられていますが、CSS セレクタで :global とか :deep を使う場合は頭にクラスをつけると影響範囲が限定されて安心できそうです
| <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> |
There was a problem hiding this comment.
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.
kitsne241
left a comment
kitsne241
left a comment
There was a problem hiding this comment.
問題なく動くことは確認しました 👍
細かい部分に変更依頼をつけたので見てもらえると嬉しいです！
| :rules="[requiredRule]" | ||
| :disabled="!question.isOpen" | ||
| variant="underlined" | ||
| class="wrap-text" |
There was a problem hiding this comment.
<style module> 式に
| class="wrap-text" | |
| :class="$style.wrapText" |
と書いて欲しいかも。他の部分をふくむ
| <style scoped> | ||
| .wrap-text :deep(.v-select__selection-text) { |
There was a problem hiding this comment.
| <style scoped> | |
| .wrap-text :deep(.v-select__selection-text) { | |
| <style module> | |
| .wrapText :deep(.v-select__selection-text) { |
CSS Modules への変更をお願いしたいです！
There was a problem hiding this comment.
moduleにするんだった。忘れてた。ありがとう！！
| :global(.v-list-item-title) { | ||
| white-space: normal; | ||
| overflow: visible; | ||
| text-overflow: clip; | ||
| } |
There was a problem hiding this comment.
Copilot による Review でも触れられていますが、CSS セレクタで :global とか :deep を使う場合は頭にクラスをつけると影響範囲が限定されて安心できそうです
kitsne241
left a comment
kitsne241
left a comment
There was a problem hiding this comment.
もう一点だけ確認をお願いしたい部分があります！ お手数おかけしてすみません
| white-space: normal !important; | ||
| } | ||
|
|
||
| :global(.v-overlay-container) :global(.v-list-item-title) { |
There was a problem hiding this comment.
| :global(.v-overlay-container) :global(.v-list-item-title) { | |
| .wrapText :global(.v-list-item-title) { |
という限定をすると影響をこのコンポーネント（QuestionEditField.vue）の中に閉じ込めることができてうれしそう
CSS Modules によって、このスタイルの .wrapText の部分がプロダクト全体で一意な文字列に置き換えられて dist に含まれることになります
There was a problem hiding this comment.
v-list-item-title(プルダウンででてくるやつ)なんですがvuetifyの仕様でteleportされてbody直下のv-overlay-containerに飛ばされるんですよね。
試してみたんですがquestion editerfieldの外で描画されていて上手くいかなかったです。
このコンポーネントとは別の箇所にcssで影響を与えるため先頭を:globalにしています。
There was a problem hiding this comment.
せめてもの制限でoverlay-containerを先頭に付けていますが別のコンポーネントでも同じようなv-selectを使うと同様に折り返しが強制されてしまいそうです。
避けるの難しいかも
|
templateを使うことでglobalに反映させることなく部分的に折り返しを反映できたので修正しました。 |
kitsne241
left a comment
kitsne241
left a comment
There was a problem hiding this comment.
LGTMです！
これは前回のレビューで言及してなくて申し訳ないのですが、QuestionShowField.vue の方にも同様の変更を入れてもらえるととても助かります
menuがbody部分に描画されるらのでglobalにcssを当てています。
タイトル部分はdeepで反映させています