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

jQueryのバージョンアップ #330

Merged
merged 13 commits into from
Nov 19, 2019
Merged

jQueryのバージョンアップ #330

merged 13 commits into from
Nov 19, 2019

Conversation

nobuhiko
Copy link
Contributor

  • yarn install
  • yarn run webpack (--watch)

jQueryのバージョンを上げる場合は

  • yarn update jquery

@coveralls
Copy link

coveralls commented Oct 25, 2019

Coverage Status

Coverage remained the same at 51.622% when pulling f641dfb on nobuhiko:version-check-js into 0c3d0da on EC-CUBE:master.

@nanasess
Copy link
Contributor

スマホや、 admin テンプレートの eccube.legacy.js はまだ必要でしょうか?

@nobuhiko
Copy link
Contributor Author

@nanasess コメントアウトすらされてなかったのでそのまま放置しましたが、消しちゃいますか?

package.json Outdated
"name": "eccube-2_13",
"version": "1.0.0",
"main": "index.js",
"repository": "git@github.com:nobuhiko/eccube-2_13.git",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"repository": "git@github.com:nobuhiko/eccube-2_13.git",
"repository": "git@github.com:EC-CUBE/eccube-2_13.git",

EC-CUBE/eccube-2_13 にしておいた方が良いかも

Copy link
Contributor Author

Choose a reason for hiding this comment

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

直しました

@nanasess
Copy link
Contributor

@nobuhiko 消しちゃっても大丈夫だと思います!

@3tiles
Copy link
Contributor

3tiles commented Oct 25, 2019

jQuery3に上げた影響か、

JQMIGRATE: jQuery.fn.change() event shorthand is deprecated

とか

JQMIGRATE: jQuery.fn.bind() is deprecated

みたいな警告が出るようです。

また、スマホだと、

JQMIGRATE: jQuery(window).on('load'...) called after load event occurred

のようなエラーが出るせいか、トップページのおすすめ商品のスライダーが動作したりしなかったりするようです。

@nobuhiko
Copy link
Contributor Author

@3tiles
ありがとうございます
警告はjquery migrate の機能なので、出ているエラーを見つつポチポチ直すしかないと思います

スマホのトップページのおすすめ商品は実機だと確かに動かないですね
jquery.flickslide.jsをちょっと見直してみます

@3tiles
Copy link
Contributor

3tiles commented Oct 27, 2019

一旦戻されるとのことなので、もう関係ないとは思いますが、折角なので(将来的な?)メモとして残しておきます。

スマホだと、商品詳細の商品画像や関連商品画像も表示されたりされなかったりしたようです。

この手の部分は詳しくないので、外しているかもしれませんが、
https://wemo.tech/109
とか参考になりそうな情報があったので貼っておきます。

あと、警告に関しては、
http://myjquery.blog.fc2.com/blog-entry-36.html
とかに情報が良く纏められているようだったのでこちらも貼っておきます。

tpl直のjsは未修正
eslintかける
@nobuhiko
Copy link
Contributor Author

商品詳細の商品画像や関連商品画像も jquery.flickslide.js が使われている場所なので同じ問題なのですが、jQuery1系なら動くけど3だと特にエラーもなく動かないので直すのがつらそうです
スマホは、jquery mobile 削除、古いプラグインも削除、っていう別の作業が必要かなと思います

@nanasess
Copy link
Contributor

jquery.flickslide.js はもうメンテされていないようなので、 4系と同じ slick とかに変更してしまっても良い気がします

ついでにcolorbox.cssもnode_modulesから読み込むように変更
不要なファイルを削除
Copy link
Contributor

@nanasess nanasess left a comment

Choose a reason for hiding this comment

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

リポジトリ名変更お願い致します🙏

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package-lock.json Outdated Show resolved Hide resolved
Co-Authored-By: Kentaro Ohkouchi <nanasess@fsm.ne.jp>
@chihiro-adachi chihiro-adachi added this to the 2.17.1 milestone Oct 29, 2019
nobuhiko and others added 2 commits October 30, 2019 10:15
Co-Authored-By: Kentaro Ohkouchi <nanasess@fsm.ne.jp>
Co-Authored-By: Kentaro Ohkouchi <nanasess@fsm.ne.jp>
@nobuhiko
Copy link
Contributor Author

意図はしてないですが #299 も直っていると思います

@chihiro-adachi chihiro-adachi merged commit 7044c94 into EC-CUBE:master Nov 19, 2019
@bbkids
Copy link
Contributor

bbkids commented Nov 22, 2019

jQueryをnode_modulesから読み込むようにしてしまった影響でしょうか、
IEからのアクセスではjQueryが読み込まれません。
Firefox, Edgeでは問題ないようですが、IEだとNGです。

@nobuhiko
Copy link
Contributor Author

@bbkids IE11で動かないの確認しました。原因はES6ですね。修正します

@bbkids
Copy link
Contributor

bbkids commented Nov 25, 2019

@babel/polyfillを追加 #346
試して見たのですが、まだNGの様です。

@nobuhiko
Copy link
Contributor Author

nobuhiko commented Nov 25, 2019

@bbkids ありがとうございます、直ったと思います。

@bbkids
Copy link
Contributor

bbkids commented Nov 25, 2019

早々の確認&修正有難う御座います。
取急ぎ、IEからのアクセスでjqueryが読み込まれるの確認できました!
直っております。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants