Opened 10 years ago
Last modified 9 years ago
#12918 review_failed Task
Kama in Retina
Reported by: | Chris Graham | Owned by: | Olek Nowodziński |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | UI : Skins | Version: | |
Keywords: | Cc: |
Description
If we contribute high quality 2x versions of the assets you have in git, would you guys take these on?
Attachments (4)
Change History (17)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
Status: | new → confirmed |
---|
comment:4 Changed 9 years ago by
Milestone: | → CKEditor 4.5.0 |
---|
comment:5 Changed 9 years ago by
Owner: | set to Olek Nowodziński |
---|---|
Status: | confirmed → assigned |
comment:7 Changed 9 years ago by
Milestone: | CKEditor 4.5.0 |
---|---|
Resolution: | → invalid |
Status: | assigned → closed |
See my comment in the original thread on GitHub.
comment:8 Changed 9 years ago by
Milestone: | → CKEditor 4.5.0 |
---|---|
Resolution: | invalid |
Status: | closed → reopened |
comment:9 Changed 9 years ago by
Milestone: | CKEditor 4.5.0 |
---|---|
Status: | reopened → confirmed |
comment:10 Changed 9 years ago by
Status: | confirmed → review |
---|
I pushed several new commits to t/12918.
comment:11 Changed 9 years ago by
Status: | review → review_failed |
---|
I rebased and pushed a commit with a manual test to the branch:t/12918.
I found couple of issues:
- kama/images/hidpi/mini.gif does not exist. A mini.png file does. A 404 error is thrown when opening any dialog. (See kama1.png)
- The "x" on the cancel button in the dialogs looks differently than before. It has a different color. (See kama1.png)
- When I built a package using kama some of the icons got a black backgrounds (2 in the forms plugin and the table). (See kama2.png)
- The Bold button is one of the most important ones and still differs much from the lowdpi one.
- Icons are not aligned vertically. If you compare kama3.pnd and kama2.png you should notice that the old icons like sub and sup or new page and preview or table and hr or numbered list and bulleted list or link and unlink. They were all well aligned. Now there's no order.
- There's something wrong with the bidi icons. (See kama2.png)
- The cut icon do would need some tweaking too.
While most of these issues does not require a lot of work I'm worried by two things:
- Number of them (and I really wasn't that picky).
- The general feeling. The low dpi icons may be blurred on Retina, but they are consistent, well aligned, lines have the same thickness and they are pretty clean. The new ones have problems in all these aspects so when I look at the whole editor something is not right.
Therefore, before we spend more time on this I want to ask if it's worth the effort. I know that a lot of work was already done (on your side Chris, but also on our), but I'm afraid that we are still far. The current state (no hidpi icons) is not that bad so I will have problems with accepting a solution which could be seen as a regression by some people.
I don't want to force this view, but my proposal is that we can fix the issues 1. and 3. and 5. and leave this branch. If anyone wants hidpi icons they can use it, so the work is not wasted. But we will not merge it to the core to avoid spending more time on the rest of issues and lowering quality of the skin.
Changed 9 years ago by
Changed 9 years ago by
Changed 9 years ago by
comment:12 Changed 9 years ago by
My point of view is that on an iMac, looking at the editor within a site that uses retina graphics, it looks very dated and out of place. While using moono, it's only really appropriate on some site designs, otherwise that looks out of place also.
So I do think lack of retina graphics is a serious issue that ages CKEditor, and hence why we invested around 2 weeks of design time into resolving it.
There are 86 icons, so that's only 1.5 hours per icon for the 2 week figure. It's not that surprising that there'd be some issues while churning through that quantity of work. We didn't use zooming in as a part of our review process on our end, or do direct comparisons between low-dpi and high-dpi, and we didn't go through your build process as it's not easy for us - but now we know these issues, I see no reason why we can't resolve them. We also weren't thinking about line thickness, as on low-dpi that is merely a function of the pixel size -- but we can't take that into account also.
If you're not going to merge, we'll have to give up, because we're not going to want to maintain a fork. But I think it is an important issue and I don't want to give up on it. We've waited such a long time between each review, and it would just be so disappointing to have to drop it all.
comment:13 Changed 9 years ago by
As I wrote, I don't want to force my view and stop you :). If you're willing to work on it, we're keep reviewing and helping.
As for maintaining this branch if it wouldn't be merge – I think it would be easy. There will not be many changes in this code in the future... If any.
Definitely! Feel free to make a pull request, I'm sure lots of Kama users will be happy seeing such improvement.