-
Notifications
You must be signed in to change notification settings - Fork 192
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
Issue #848 - improve layout CSSFixme #891
Conversation
@magsout what does code that has been modified look like? See http://hallvord.com/temp/moz/cssfixme.php?url=http%3A%2F%2Fhallvord.com%2Ftemp%2Fmoz%2FcompatTesterTesting%2Fstyle.css for an example. |
Very minor language point: I wonder if "CSS code" will be easier to understand than "CSS instructions"..? |
@hallvors aaaaahh, ok, 👎 my bad... So, I return to the previous style for the |
@hallvors @karlcow @miketaylr Which one do you prefer ? |
@magsout The 1st one (at the top). Many thanks. Super travail! |
@hallvors is it possible to detect if there is content in the |
We can certainly set an is-active style from the JS when inserting the generated code. Or just generate the PRE element too on the fly..? You can just add a DIV id=result-parent or something to put the generated PRE inside, I think that may be cleaner than toggling PRE style based on a class :) |
(I also vote for the 1st one - much of the point is to show very clearly what we're changing and it's more obvious that way) |
@hallvors looks good 👍 |
- removed <pre> which is generated by the script
@@ -20,7 +28,13 @@ | |||
<button type="button" class="Button Button--default" id="btn_do_fixup">Add standard equivalents for prefixed CSS</button> | |||
</div> | |||
<div class="wc-CSSFixme-item"> | |||
<pre class="wc-CSSFixme-resultat" id="fixedcss"></pre> | |||
<div id="js-CSSFixme"></div> |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
Ok, updated with the style for pre |
build passed |
Same, top style looks great. |
I sort of liked the way the code in the second screenshot stands out with the yellow background though.. I'm testing the JS changes now, when the background is plain it is too much a part of the page, somehow. What about adding for example
To make it stand out a bit more? |
@magsout I hope you're happy with me pushing directly here..? |
I'm about to push a fix for that. I don't understand 100%, but the |
(This fixes a double script include bug)
1 similar comment
oups .. ;) |
Great, now we can undo the global-ness of that variable. If I do that quickly enough perhaps I can make it before Mike merges ;) |
and @magsout 👍 to the extra styling - stands out now |
Heh, I'll let you pull the merge trigger @hallvors -- and I can deploy this afternoon as well. |
Issue #848 - improve layout CSSFixme
Added some improvement:
r? @karlcow @hallvors