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

グロナビのヘルプをヘッダーに移動 #7400

Merged
merged 9 commits into from
Mar 9, 2024

Conversation

ghost
Copy link

@ghost ghost commented Feb 17, 2024

Issue

概要

ヘルプをヘッダーに移動し、管理者ユーザーの場合は「受講生用ヘルプ」と、「アドバイザー用ヘルプ」をドロップダウンの中から選べるように変更

machidaさんのコメントも参照

変更確認方法

  1. feature/combine-advisor-help-and-help-for-adminをローカルに取り込む
  2. adminユーザーでログインする
  3. ヘッダー内の「ヘルプ」をクリックすると、「受講生用ヘルプ」と、「アドバイザー用ヘルプ」が表示され、それぞれのページへのリンクになっていることを確認する

Screenshot

変更前

image

変更後

image

@ghost ghost assigned machida and ghost Feb 17, 2024
@ghost
Copy link
Author

ghost commented Feb 17, 2024

  1. 初期状態では、リンクのリストulは非表示(display: none)になっている。

image

  1. ヘルプをクリックすると、リンクのリストが表示される(display: block)が、現在グロナビを右にスクロールしないと見えない状態になってしまっている。(この状態になるとグロナビの下部にスクロールバーが表示される)

image

  1. グロナビを右にスクロールすると、ヘルプページとアドバイザーヘルプページへのリンクのテキストが表示されているのが確認できる。(現在文字色と背景色が同じになっているため、全選択で文字が見えるようにしている)

image

  1. リンクのリストul以外の要素をクリックすると、リストは非表示になる。

image

@ghost
Copy link
Author

ghost commented Feb 17, 2024

@machida
お疲れ様です。
#7386 の動作の部分を作成したのですが、現在上記のコメントのような状態になっています(グロナビの幅が広がってスクロールバーが表示され、main要素の上に出るような形で表示されない)。
こちらはデザインの方で対応していただく箇所という認識で大丈夫でしょうか?
問題がなければ、こちらのデザインをお願いします。
変更を加えた個所に何か問題があれば教えていただけると助かります🙇‍♂️

@machida
Copy link
Member

machida commented Feb 19, 2024

@taco-nantai 対応ありがとうございます!これでデザイン入れられますー🙆‍♂️

@machida machida force-pushed the feature/combine-advisor-help-and-help-for-admin branch from 6312631 to cf770c3 Compare February 28, 2024 08:13
@machida
Copy link
Member

machida commented Feb 28, 2024

@taco-nantai

ちょっと変更を加えデザインを整えました。

変更箇所

1

image

ポートフォリオがグロナビに追加され、グロナビが縦に長くなってしまったので、ヘルプをヘッダーに移動しました。

2

子メニューの表示・非表示のJSをtacoさんのものを少しいじってヘルプ、Me、モバイル時のメニュー(つまり通知以外)で共通化させました。

通知に関しては、#7257 このPRが近いうちにマージされるので、通知は共通化していません。

今回の変更で、

image

このように通知のときだけ子メニューが開くと背景が黒くなりますが、
#7257 このPRをマージしてから、全て背景を黒くするか、黒くするのを辞めるか、どっちかに揃える予定です。

@ghost ghost changed the title 管理者のグロナビのヘルプを一つに統合 ヘルプをヘッダーに移動 Mar 2, 2024
@ghost ghost changed the title ヘルプをヘッダーに移動 グロナビのヘルプをヘッダーに移動 Mar 2, 2024
@ghost ghost marked this pull request as ready for review March 2, 2024 02:48
@ghost ghost requested a review from yocchan-git March 2, 2024 22:52
@ghost
Copy link
Author

ghost commented Mar 2, 2024

@yocchan-git
お疲れ様です。
こちらのPRのレビューをお願いしてもよろしいでしょうか🙏
お手すきの際で構いませんので、よろしくお願いいたします🙇‍♂️

Copy link
Contributor

@yocchan-git yocchan-git left a comment

Choose a reason for hiding this comment

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

動作はOKそうです!
コンフリクトしてそうなので、下記を参考にmainrebaseして解消していただけますと🙏

https://bootcamp.fjord.jp/pages/305#push

解消しましたら声かけください!
今日は用事がありまして、今日はもう対応できませんが明日の朝に対応させていただきます🙇‍♂️

@ghost
Copy link
Author

ghost commented Mar 3, 2024

@machida
お疲れ様です。
デザインの修正ありがとうございます。
現在、編集していただいたapp/javascript/notifications_bell.vueapp/views/application/header/_header_links.html.slimでコンフリクトが発生しているようなのですが、これらのファイルに関して、このブランチで編集していただいたものとmainブランチのもの、どちらを残せばよいでしょうか。
お手数をおかけしますが、ご教授いただければ幸いです🙇‍♂️

@machida machida force-pushed the feature/combine-advisor-help-and-help-for-admin branch from 8048a87 to 99aa323 Compare March 5, 2024 06:57
@machida
Copy link
Member

machida commented Mar 5, 2024

@taco-nantai 最新のmainを取り込んでコンフリクトを解消しておきましたー

git pull --rebase origin feature/combine-advisor-help-and-help-for-admin

をしておいてくださいー

@machida machida removed their assignment Mar 5, 2024
@ghost
Copy link
Author

ghost commented Mar 5, 2024

@machida
ありがとうございます🙇‍♂️

@ghost ghost requested a review from yocchan-git March 5, 2024 10:22
@ghost
Copy link
Author

ghost commented Mar 5, 2024

@yocchan-git
お疲れ様です。
machidaさんにコンフリクトの解消をしていただいたので、改めてレビューをお願いしてもよろしいでしょうか。
お手すきの際で構いませんので、よろしくお願いいたします🙇‍♂️

Copy link
Contributor

@yocchan-git yocchan-git left a comment

Choose a reason for hiding this comment

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

LGTM🎉
動作確認・コード共に問題なさそうです!

テストはflakyなやつが落ちてそうでしたので、通ればkomagataさんへ
対応お疲れ様でした〜〜🍵

@ghost ghost requested a review from komagata March 5, 2024 16:59
@ghost
Copy link
Author

ghost commented Mar 5, 2024

@komagata
お疲れ様です。
PRにapproveをいただいたので、レビューをお願いします🙇‍♂️

Copy link
Member

@komagata komagata left a comment

Choose a reason for hiding this comment

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

確認させて頂きました。OKです〜🙆‍♂️

@komagata komagata merged commit af94fba into main Mar 9, 2024
5 checks passed
@komagata komagata deleted the feature/combine-advisor-help-and-help-for-admin branch March 9, 2024 16:48
@github-actions github-actions bot mentioned this pull request Mar 9, 2024
29 tasks
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.

3 participants