llaske / ExerciserReact

React code of Exerciser Activity. GSoC 2018
Apache License 2.0
15 stars 49 forks source link

Add and delete button bootstrap issue fixed. #34

Closed AvinashAgarwal14 closed 5 years ago

AvinashAgarwal14 commented 5 years ago

Fixes: #33 Before: up_down_before

After: up_down_after

AvinashAgarwal14 commented 5 years ago

@llaske Could you please review this PR?

AvinashAgarwal14 commented 5 years ago

@llaske Please review this PR and let me know what changes you expect?

llaske commented 5 years ago

It solve the add and delete button issue but other buttons are white.

Capture d’écran de 2019-04-21 21-32-13

It seems it don't happens on every computer. May be it depends of bootstrap?

AvinashAgarwal14 commented 5 years ago

On my system, the fix works just fine. I will test it on different systems and try to come up with a solution. Can you please tell me which browser are you using and its version.

llaske commented 5 years ago

It's Chromium Version 75.0.3756.0 on Ubuntu 18.04.2

AvinashAgarwal14 commented 5 years ago

@llaske I think the last commit should solve the issue. Please review the PR. Thanks :)

llaske commented 5 years ago

Sounds good but it's now the same problem during run time for the "submit" button. Plus it should be fixed for other templates too. Could you explain why it works on some browsers and not on others?

AvinashAgarwal14 commented 5 years ago

@llaske Some browser's are recognizing background-color: transparent as bootstrap's default btn CSS, while others are not. This problem is happening in those browsers for which the buttons default background colour is recognized as transparent. So, for all those button which are initially disabled (background-color: light-gray!important property is added) as soon as they are enabled instead of changing the colour to deep grey, the transparent property is given high priority. Hence the background of such button appears white. This problem is solved by providing important flag to background-color: #808080!important to all those buttons which are initially disabled. There is no need for any fix in case of those browsers which do not recognize bootstrap's default btn background-color as transparent. Although I don't know why there is a difference in bootstrap's CSS property of buttons in different browsers.

I think I have added the important flag in all such buttons. Please let me know if I missed one. Thanks :)

llaske commented 5 years ago

Good job. Thanks for the explanation. BTW it change the text color on button (now black instead of white) plus now the color don't change when the mouse is over the button.

AvinashAgarwal14 commented 5 years ago

@llaske Please review the changes I made in the last commit and let me know if I missed something. Thanks :)

llaske commented 5 years ago

😮 Ooch you can't change sugar-96dpi.css and sugar-200dpi.css, these files are used in all activities.

AvinashAgarwal14 commented 5 years ago

@llaske Sorry. I have reverted those changes and added new ones. Please review.

llaske commented 5 years ago

Very good. Thanks.