search

phi-jp / meltline

melt + inline. css library
0 stars 3 forks source link

SNS用のカラーを追加 #49

Closed hayato-jp closed 4 years ago

hayato-jp commented 4 years ago
kinchan-jp commented 4 years ago

3色追加？

Iruru-jp commented 4 years ago

@kinchan-tokyo numsみたいにcolorをtext-〇〇, bg-〇〇, border-〇〇に当てはめてeachしてる感じですね！

Iruru-jp commented 4 years ago

LGTM

snuffy commented 4 years ago

4 * 166 = 664 個クラスが　ふえるな。。。 @simiraaaa これパフォーマンスに影響でないでしたっけ？

simiraaaa commented 4 years ago

@snuffy 多少は影響出るけど、そもそも、4色ぐらいしか追加しないはずだからコードが違う

phi-jp commented 4 years ago

@snuffy @simiraaaa コードは合ってる! @snuffy の理解が違うね

既存の色が配列だったのをオブジェクトにしただけで色の追加量自体は @simiraaaa のいうとおりSNSカラー分しか増えてない

snuffy commented 4 years ago

@simiraaaa @phi-jp

ありゃ、すみません！完全に勘違いしてましたー

確かにであれば、LGTMです！

phi-jp commented 4 years ago

@hayato-jp これちゃんと動作確認してないよね？ npm run dev で build できてるかちゃんと確認してから pull req 投げてくれ

昔の指定が残っちゃってる

.for(@colors, {
hayato-jp commented 4 years ago

@hayato-jp これちゃんと動作確認してないよね？ npm run dev で build できてるかちゃんと確認してから pull req 投げてくれ

昔の指定が残っちゃってる

.for(@colors, {

@phi-jp すみません、npm run devしつつ作業はしていたのですが、プルリク出す手前に確認が出来ていませんでした。以後気をつけます。

昔の指定なのですが、コメントアウトの形で残した方がいいでしょうか？ 残しておいた理由と致しましては、既存のdefine-colorは残すと指示を貰い、そのdefine-colorに色を回すための .for(@colors, { も残した方がいいと思って残していました。エラーが出てしまっていたのは確認ができていなかったためでした。すみません。

phi-jp commented 4 years ago

@hayato-jp コメントアウトで良いよ👍

hayato-jp commented 4 years ago

@hayato-jp コメントアウトで良いよ👍

@phi-jp 修正しました！確認お願い致します🙇‍♂️

kasaji-jp commented 4 years ago

LGTM

Iruru-jp commented 4 years ago

改めてLGTM

hayato-jp commented 4 years ago

マージします