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

Change the color of color-picker icon even when the text tool is selected #11642

Merged
merged 4 commits into from Mar 15, 2021

Conversation

hiroshisuga
Copy link
Contributor

@hiroshisuga hiroshisuga commented Mar 13, 2021

What does this PR do?

Let the color of the color-picker icon be changed even when the text tool is selected

Closes Issue(s)

closes #11641

Motivation

I think the color of the color-picker and that of the annotation icon should always be the same to avoid confusion.

More

In the current implementation, both this.thicknessListIconRadius and this.thicknessListIconColor are null when the text tool is selected. So "if (this.thicknessListIconRadius && this.thicknessListIconColor)" never happens as long as the text tool is selected. I don't think it's the intended behaviour.

Anyway, this is what happens when this modification is applied:
toolbar2

@@ -283,15 +283,15 @@ class WhiteboardToolbar extends Component {
* 4. Trigger initial animation for the icons
*/
// 1st case
if (this.thicknessListIconRadius && this.thicknessListIconColor) {
if ( (this.thicknessListIconRadius && this.thicknessListIconColor) || annotationSelected.value === 'text') {
if (colorSelected.value !== prevState.colorSelected.value) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another solution is to completely remove this condition, which I confirmed to work as well.

Copy link
Collaborator

@KDSBrowne KDSBrowne left a comment

Choose a reason for hiding this comment

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

Tested, works well.

@antobinary
Copy link
Member

Thanks @hiroshisuga and @KDSBrowne !

@antobinary antobinary merged commit cabddc6 into bigbluebutton:develop Mar 15, 2021
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.

WB toolbar text color change does not alter the color-picker icon
3 participants