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

Improvement needs on hover of "Ok" button of source code modal on product page #11424

Merged
merged 1 commit into from Nov 20, 2018

Conversation

Projects
None yet
5 participants
@dineshbadrukhiya
Contributor

dineshbadrukhiya commented Nov 17, 2018

Questions Answers
Branch? develop
Description? Improvement needs on hover of "Ok" button of source code modal on product page
Type? bug fix
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #11216
How to test? Hover the "Ok" button of source code modal on product page

This change is Reviewable

@prestonBot

This comment has been minimized.

Collaborator

prestonBot commented Nov 17, 2018

Hello @dineshbadrukhiya!

This is your first pull request on the PrestaShop project. Thank you, and welcome to this Open Source community!

@matks

This comment has been minimized.

Contributor

matks commented Nov 19, 2018

Thank you for your contribution 😄 looks good to me but since CSS is not my best I'll wait for one of my peers' review too

@dineshbadrukhiya

This comment has been minimized.

Contributor

dineshbadrukhiya commented Nov 19, 2018

@matks As you have suggested I will commit with compiled theme.css 😀

@Quetzacoalt91

After a short investigation, it seems we can get rid of another !important style definition:

.mce-btn:focus, .mce-btn:hover {
background-color: transparent !important;
}

My opinion would be removing these lines instead.

min-width: 50px
color: #fff
border: 1px solid #b1b1b1
border-color: rgba(0, 0, 0, 0.1) rgba(0, 0, 0, 0.1) rgba(0, 0, 0, 0.25) rgba(0, 0, 0, 0.25)
background-color: #006dcc
&:hover, &:focus
background-color: #005fb3
background-color: #005fb3 !important

This comment has been minimized.

@Quetzacoalt91

Quetzacoalt91 Nov 19, 2018

Member

Hmm, I totally dislike the !important, it prevents people to update the value with another css file.

In my opinion the change to apply must be done elsewhere.

This comment has been minimized.

@dineshbadrukhiya

dineshbadrukhiya Nov 20, 2018

Contributor

Hello @Quetzacoalt91
I had already tried to modify js/tiny_mce/skins/prestashop/skin.min.css by removing line as you have suggested. But when I go to back-office, removed css style still there after clearing cache also. So I thinked that it's loads by default with js :) and then tried to modify admin-dev/themes/new-theme/scss/components/_tinymce.sass.

I also dislike to use !important but here I need to override the style .mce-btn:focus, .mce-btn:hover { background-color: transparent !important; } from js/tiny_mce/skins/prestashop/skin.min.css file. That's why I have used style with !imortant.

This comment has been minimized.

@Quetzacoalt91

Quetzacoalt91 Nov 20, 2018

Member

I managed to make it work on my side, did you make sure your browser cache was emptied?

This comment has been minimized.

@dineshbadrukhiya

dineshbadrukhiya Nov 20, 2018

Contributor

@Quetzacoalt91 I have cleared browser cache and it's fixed for me now.

This comment has been minimized.

@dineshbadrukhiya

dineshbadrukhiya Nov 20, 2018

Contributor

@Quetzacoalt91 Thanks. I would like to thank you @matks also as you helped me a lot on Gitter regarding this PR 😀

This comment has been minimized.

@Quetzacoalt91

Quetzacoalt91 Nov 20, 2018

Member

And now you can remove the changes applied in this file and admin-dev/themes/new-theme/public/theme.css, as they are properly applied on the popup correctly. :)

This comment has been minimized.

@dineshbadrukhiya

dineshbadrukhiya Nov 20, 2018

Contributor

@Quetzacoalt91 Does I need to remove old commits as I'm not good to remove it :).

@Quetzacoalt91 Quetzacoalt91 force-pushed the dineshbadrukhiya:Fix-#11216 branch from 6e750c4 to 744066f Nov 20, 2018

@Quetzacoalt91 Quetzacoalt91 added this to the 1.7.6.0 milestone Nov 20, 2018

@marionf marionf added QA ✔️ and removed waiting for QA labels Nov 20, 2018

@Quetzacoalt91 Quetzacoalt91 merged commit 22c5347 into PrestaShop:develop Nov 20, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Quetzacoalt91

This comment has been minimized.

Member

Quetzacoalt91 commented Nov 20, 2018

Thank you @dineshbadrukhiya

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment