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

Design修正-2 #71

Merged
merged 27 commits into from
Mar 12, 2021
Merged

Design修正-2 #71

merged 27 commits into from
Mar 12, 2021

Conversation

naokikubo2
Copy link
Owner

@naokikubo2 naokikubo2 commented Mar 10, 2021

概要

前PRに引き続き、デザインの修正と、機能面の修正を行いました。

該当issue

参考資料

うまく行っていない事象

概要

  • 日時のカレンダー入力フォームが正常に動作しない時があります。
    • はじめてカレンダー入力フォームのあるページに遷移した時 ⇨ 動作しない
    • リロードして表示した時 ⇨ 動作する
      ( 動作するとは、カレンダーアイコンをクリックしたときに、カレンダーが表示されることを表しています。)

カレンダー表示に使用している技術

解決方法

  • 資料B こちらに記載されているようにライブラリ自体に問題があり、変数configのnullチェックを追加するという解決方法があります。

聞きたいこと

コンテナ内のjsファイルを修正することになると思うのですが、
./qs bash web でコンテナに入り、vimでファイル修正をするというやり方で合っていますでしょうか。ライブラリを修正するということをしたことがないので、実施前に確認したいです。

@naokikubo2 naokikubo2 requested a review from belion-freee March 10, 2021 01:03
Comment on lines +47 to +59
<div class="input-group date" id="datetimepicker1" data-target-input="nearest">
<input type="text" class="form-control datetimepicker-input" data-target="#datetimepicker1" name="q[food_date_gteq]" id="q_food_date_gteq"/>
<div class="input-group-append" data-target="#datetimepicker1" data-toggle="datetimepicker">
<div class="input-group-text"><i class="fa fa-calendar"></i></div>
</div>
</div>
<div>~</div>
<div class="input-group date" id="datetimepicker2" data-target-input="nearest">
<input type="text" class="form-control datetimepicker-input" data-target="#datetimepicker2" name="q[food_date_lteq]" id="q_food_date_lteq"/>
<div class="input-group-append" data-target="#datetimepicker2" data-toggle="datetimepicker">
<div class="input-group-text"><i class="fa fa-calendar"></i></div>
</div>
</div>
Copy link
Owner Author

Choose a reason for hiding this comment

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

var script = document.createElement('script');
script.src = 'https://maps.googleapis.com/maps/api/js?key=' + process.env.GOOGLE_CLOUD_API + '&callback=initMap';
script.defer = true;
import { Loader } from '@googlemaps/js-api-loader';
Copy link
Owner Author

Choose a reason for hiding this comment

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

https://developers.google.com/maps/documentation/javascript/overview#js_api_loader_package
上記を参考に、inline loadingからdynamic loadingに変更。

@@ -22,7 +22,7 @@ def store_dir
# end

# Process files as they are uploaded:
process resize_to_fit: [200, 300]
process resize_to_fill: [300, 300, "Center"]
Copy link
Owner Author

Choose a reason for hiding this comment

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

https://qiita.com/wann/items/c6d4c3f17b97bb33936f
上記を参考に、一辺300pxの正方形を切り抜くことにした。

@belion-freee
Copy link
Collaborator

belion-freee commented Mar 10, 2021

日時のカレンダー入力フォームが正常に動作しない時があります。
はじめてカレンダー入力フォームのあるページに遷移した時 ⇨ 動作しない
リロードして表示した時 ⇨ 動作する
( 動作するとは、カレンダーアイコンをクリックしたときに、カレンダーが表示されることを表しています。)

事象的にturbolinksが原因の可能性があるので、一度外して再度検証してもらえますか?
外しても大して影響はないです。むしろ外さないで意図しない挙動になるデメリットの方が大きい。
railsサーバーの再起動も忘れずに。
https://qiita.com/bambis13s/items/606ff32b07e4eb61cdb0

上記でも事象が再現される場合は、下記のコメントの対応を試してみてください。
tempusdominus/bootstrap-4#193 (comment)

あと、参照してるライブラリのStarが少ないのと、14カ月間更新ないのがちと不安ではありますね。Issue見るとこのgemを採用したのはモバイル端末でのみカレンダー入力がうまくいかなかったからとのことですが、他に問題を解決すべきライブラリはなさそうだったと言うことですかね。
https://github.com/platonic7/datetimepicker

Copy link
Collaborator

@belion-freee belion-freee left a comment

Choose a reason for hiding this comment

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

jsが動かない件以外は大きな問題なさそうです。

Comment on lines 57 to 64
find_or_create_by!(email: 'email1@com') do |user|
user.password = SecureRandom.urlsafe_base64
# user.confirmed_at = Time.now # Confirmable を使用している場合は必要
user.location_id = 1_850_147
user.address = '東京'
user.latitude = 35.7090259
user.longitude = 139.7319925
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

外部から入力されたパラメーターで作るのでないなら、seedを使ってguestユーザーを作っておいた方がいいかと思います。

Copy link
Owner Author

Choose a reason for hiding this comment

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

確かにそうですね。
ありがとうございます。対応します。

@naokikubo2
Copy link
Owner Author

  • カレンダー入力の件ですが、turbolinksを除去することで、正常に動作させることができました。今回、カレンダー入力のライブラリにこちらを使った理由としては、「時間も入力可能」「bootstrapと親和性がある」「webpacker管理」という観点で調べたところ、こちらが見つかり、試したところ問題なさそうだったので採用しました。

  • また、turbolinks除去に伴い画像プレビューがturbolinksを想定して発火させていたので、そちらを修正しました。

@belion-freee
Copy link
Collaborator

カレンダー入力の件ですが、turbolinksを除去することで、正常に動作させることができました。

よかったです。「この機能を外すことがRailsアプリ開発の第一歩」と揶揄されるくらい微妙な機能なんですよね。

今回、カレンダー入力のライブラリにこちらを使った理由としては、「時間も入力可能」「bootstrapと親和性がある」「webpacker管理」という観点で調べたところ、こちらが見つかり、試したところ問題なさそうだったので採用しました。

了解です。今回はポートフォリオなので問題ないですが、現場だと保守していく必要があるので、ライブラリの選定などの条件に、「定期的にメンテナンスされてるか?」、「広く使われていて信用性に足りるか?」と言う点は入るので、意識だけしておいてください。

Copy link
Collaborator

@belion-freee belion-freee left a comment

Choose a reason for hiding this comment

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

LGTM

@naokikubo2
Copy link
Owner Author

了解です。今回はポートフォリオなので問題ないですが、現場だと保守していく必要があるので、ライブラリの選定などの条件に、「定期的にメンテナンスされてるか?」、「広く使われていて信用性に足りるか?」と言う点は入るので、意識だけしておいてください。

これらの観点が不足していました。
保守性・信用性を、技術の選定時の観点として取り入れたいと思います。ありがとうございます。

@naokikubo2 naokikubo2 merged commit 6e460e2 into master Mar 12, 2021
@naokikubo2 naokikubo2 added the phase4 In this phase we develop UI/UX label Mar 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
phase4 In this phase we develop UI/UX
Projects
None yet
2 participants