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

fix phina.util.Color #223

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

ihoronir
Copy link
Contributor

@ihoronir ihoronir commented Nov 19, 2017

変更点

  • phina.util.ColorのhslaのMATCH_SET_LISTがrgbaになっている #220 を修正 setFromString()setSmartでhslaがうまく動かないのを修正
  • phina.util.ColorクラスのsetSmart()のバグ #221 を修正 setSmart()で数値からr, g, bを指定できないバグを修正
  • " を ' に変更
  • == を === に変更
  • set(), setFromNumber()の仕様変更
    • それに伴い、setFromNumber()を使用しているメソッドの仕様も変更
    • 具体的には引数が見つけられなかった場合にr, g, b, aを変えないようにした。
      • コンストラクターに引数がなかったときにr, b, gnullになるのを防ぐことができる。
      • setFromObject() では任意の値だけ変更できて便利
  • Webカラー140色を追加
  • HSLtoRGB()s===0だったときに[0, 0, 0]を返すように変更 勘違いしてたので修正しました{693d4c9}  一応、中で使われてる変数名を、引数と同じlではなくnに変更しました。
  • createStyleRGBのバグを修正

今後検討するべきだと思うこと

  • setFromString() で RGB など Alpha の情報がなかった場合、a を 1 にすべきか、元の状態のままにすべきか。
    • set()ではaを省略してもaが1になることはないが、そのこととの整合性をどうするか。
      • set()でもaが省略されたとき、1にリセットされる方が良い?それとも1にリセットされない方が良い?

@phi-jp
Copy link
Collaborator

phi-jp commented Nov 20, 2017

@shioleap

カラーリスト増えてるw
コード整理ありがとうございます!!

setFromString() で RGB など Alpha の情報がなかった場合、a を 1 にすべきか、元の状態のままにすべきか。

これは迷いどころですね.

@ihoronir
Copy link
Contributor Author

ihoronir commented Nov 20, 2017

個人的にはaを1にリセットする方が分かりやすい気がします!

this.a = (a !== undefined) ? a : 1.0;
this.r = (r === 0) ? 0 : r || this.r;
this.g = (g === 0) ? 0 : g || this.g;
this.b = (b === 0) ? 0 : b || this.g;
Copy link
Contributor

Choose a reason for hiding this comment

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

(b === 0) ? 0 : b || this.g;ここthis.bですね

@ihoronir
Copy link
Contributor Author

ihoronir commented Dec 8, 2017

@simiraaaa さんにご指摘いただいたtypo修正いたしました!
ついでと言っては何ですが、setFromString() で RGB など Alpha の情報がなかった場合、a を 1 にす
るようにしました。

@simiraaaa
Copy link
Contributor

@shioleap LGTM

@phi-jp 自分はこれで問題ないと思います。

@ihoronir ihoronir force-pushed the feature/fix-phina-util-color branch from ffce78e to 8359d90 Compare December 18, 2017 14:04
@ihoronir
Copy link
Contributor Author

rebaseしました

@ihoronir ihoronir force-pushed the feature/fix-phina-util-color branch from 8359d90 to 04fee02 Compare February 8, 2021 09:22
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