-
Notifications
You must be signed in to change notification settings - Fork 597
Update for thread colors #785
base: master
Are you sure you want to change the base?
Conversation
////Fern: "#67b868", | ||
////Cameo: "#d4a88c", | ||
////BrilliantRose: "#ff5ca1", | ||
////BilobaFlower: "#a695c7" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these the same colors with different names? This is a really breaking change...
Everything else looks great but I wonder if we can keep some of the same names?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
umm, those are some old color which have been commented. also facebook uses theme id now, so those old colors can't be used anymore.
and no, we can't keep them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that the api has changed, but clearly some of these colors must be similar MessengerBlue
vs DefaultBlue
for example.. I remember the original names came from the messenger code somewhere, where did you get the new names?
I'm sure you understand that this change break anyone who currently uses this api
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lequanglam you should put new ids in place of old hex colours (instead of commenting them). This way we can maintain backward compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ravkr I will try to make it backward-compatible, but i'm currently busy at listenMqtt's bugs.
Resolves #784