Feature/modal destroy#25 #94
Conversation
…hanged to selectable.
src/core/index.js
Outdated
| const height = Number(arr[1]); | ||
| const padding = height * 100 / width; | ||
| return `${padding}%`; | ||
| isModal(event) { |
There was a problem hiding this comment.
@1000-x-t30
個人的な意見ですが、isModal というメソッド名だと モーダルかどうかを判別するメソッドなのかな？と思ってしまいますので、他の名前にしたほうがいいかと感じました。
例えばよくあるのは onClick や handleClick などです。
ご検討よろしくお願いします。:bow:
src/core/index.js
Outdated
| getHtml(opt, videoUrl, id) { | ||
| const padding = this.getPadding(opt.ratio); | ||
| const classNames = opt.classNames; | ||
| reAdd() { |
There was a problem hiding this comment.
@1000-x-t30
メソッド名を re（再度という意味） をなくした名前に変更（create や mount など）。
constructor 内でも同じメソッドを利用したいので、re というのはおかしくなってくるため。
src/core/index.js
Outdated
| const padding = this.getPadding(opt.ratio); | ||
| const classNames = opt.classNames; | ||
| reAdd() { | ||
| this.selectors = typeof this.selectors === 'string' ? document.querySelectorAll( this.selectors) : this.selectors; |
There was a problem hiding this comment.
@1000-x-t30
this.selectors を更新するかどうかは引数によって変える。
src/core/index.js
Outdated
| } | ||
|
|
||
| destroy() { | ||
| this.selectors = typeof this.selectors === 'string' ? document.querySelectorAll( this.selectors) : this.selectors; |
There was a problem hiding this comment.
@1000-x-t30
destory は this.selector の再取得はなくてもok
src/core/index.js
Outdated
| if(eacquisition) { | ||
| this.selectors = typeof this.selectors === 'string' ? document.querySelectorAll( this.selectors) : this.selectors; | ||
| } | ||
| console.log(this.selectors) |
There was a problem hiding this comment.
@1000-x-t30
デバックコードは削除お願いします。！！:bow:
src/core/index.js
Outdated
| getHtml(opt, videoUrl, id) { | ||
| const padding = this.getPadding(opt.ratio); | ||
| const classNames = opt.classNames; | ||
| add(eacquisition = false) { |
There was a problem hiding this comment.
@1000-x-t30
引数名 isUpdateSelectors とかにしませんか？？
あまりプログラムの変数で aeacquisition 利用しない気がするので isUpdateSelectors とかのほうが直感的ででわかりやす行かなと思いました。
I was able to work on it. Please confirm. #25 #41