#13119 closed Task (fixed)
Improve skins' CSS resets
Reported by: | Piotrek Koszuliński | Owned by: | Piotrek Koszuliński |
---|---|---|---|
Priority: | Normal | Milestone: | CKEditor 4.5.0 |
Component: | General | Version: | |
Keywords: | Cc: |
Description
The new samples proved that the current CSS reset rules are insufficient. For instance - margin of sourcearea's textarea, underline and color of links.
Attachments (2)
Change History (15)
comment:1 Changed 10 years ago by
Status: | new → confirmed |
---|
comment:2 Changed 10 years ago by
Owner: | set to Piotrek Koszuliński |
---|---|
Status: | confirmed → assigned |
comment:4 Changed 10 years ago by
Milestone: | CKEditor 4.5.0 Beta |
---|
We changed samples framework styling a little bit, so the issues that we had are gone. I'm removing milestone, but I still wait for your feedback.
Changed 10 years ago by
Attachment: | basic_sample_source_code_styling.png added |
---|
Changed 10 years ago by
Attachment: | Screen Shot 2015-04-21 at 14.51.26.png added |
---|
comment:7 Changed 10 years ago by
WFM (see http://dev.ckeditor.com/attachment/ticket/13119/Screen%20Shot%202015-04-21%20at%2014.51.26.png - I can't attach it in this comment because Trac can't deal with spaces :D)
comment:9 Changed 10 years ago by
And the problem is that it has the default display:inline, while it should have display:block to be independent on line heights and font sizes.
comment:10 Changed 10 years ago by
Status: | assigned → review |
---|
I pushed more fixes to branch:t/13119. Both skins are improved.
comment:11 Changed 10 years ago by
Status: | review → review_passed |
---|
Even if something's wrong with the changes, we'll get a chance to spot it in the course of development. So better merge it sooner than later.
comment:12 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | review_passed → closed |
Merged into major in git:2e0eee9.
comment:13 Changed 10 years ago by
Type: | Bug → Task |
---|
I've been thinking about this and I've got few observations:
#content div { float: left; border: solid 5px red }
it's his problem..cke_reset_all * {}
has lower specificity than.mycontent a {}
... That was really surprising for me and it's sad because it makes our reset rules less significant than usual.mycontent sometag {}
styles.In my opinion links cause the biggest problem. We don't have right not a rule with specificty 11+ which clean them. Another tag which I saw touched is the sourceare and that's because it's not inside any
cke_reset_all
element.I pushed branch:t/13119 with a manual test in which I styled content accordingly to the above rules - that we can't be bulletproof, but we can do better with specific elements rules (I chose only link and textarea) which have specificity 11 and set some reasonable (popular) styles.
I also made necessary changes to our reset.css (added
.cke_reset_all a, .cke_reset_all textarea
rules) and tuned related rules so they have specificity of 11+ too.WDYT about the above? Please remember that the patch is not meant to solve all problems, but simply improve the situation for most common scenarios.
PS. This small research was pretty interesting and was a good lesson that we could use when working on CKEditor 5 or any new skin that we'll build. It proves that CSS reset should be based on rules of specificity 11+ for as many elements that we use in our UI as possible. It can't be done for moono or kama now, because if I added
.cke_reset_all span
for instance it would make all other rules based on classes only (specificty = 10) ignored.