Opened 5 years ago

Closed 5 years ago

#11757 closed Bug (fixed)

Imperfections in moono ui styles

Reported by: Danil Owned by: Olek Nowodziński
Priority: Normal Milestone: CKEditor 4.4.2
Component: UI : Skins Version: 4.0
Keywords: Cc:

Description

  • "Browse" button has wrong top margin.
  • Combobox has cursor:pointer over an arrow and borders.
  • Specialchars table has cells of different widths.
  • Dialog buttons and selects has 24px height, while textfields 25px
  • "Close" button background is not centered. The button has no "hover" effect.

Change History (7)

comment:1 Changed 5 years ago by Danil

I made a pull request, but don't know how to link it properly to this ticket: https://github.com/ckeditor/ckeditor-dev/pull/91

comment:2 Changed 5 years ago by Jakub Ś

Status: newconfirmed
Version: 4.3.5 (GitHub - master)4.0

This is something we definitely can improve.

comment:3 Changed 5 years ago by Olek Nowodziński

Owner: set to Olek Nowodziński
Status: confirmedreview

I reviewed ​https://github.com/ckeditor/ckeditor-dev/pull/91. A few remarks here:

I'm OK with cursor:default for groups inside of the combo (.cke_panel_grouptitle). Nice catch. Also I find cursor:default right for the drop-down arrow in the toolbar since it was the only UI element in the toolbar that changed cursor on hover.

This change has no impact or it's negligible. Rejected.

IE fix (i.e. IE8). It's correct.

  • git:1c9d199 Adjust find dialog field sizes (mostly for non-english languages)

I'm not able to reproduce any issue here. Also that commit brings no visual change for me. Rejected.

Nice catch. It's correct.

This one is also fine.

  • git:2f86e66 Center x in close button, add a slight hover effect to it

That's true. "x" was not centered right. As for the hover effect, it's an eyecandy and I like it.

Generally speaking I feel great about this PR. Kudos to danya_postfactum. I pushed branch:t/11757 which does not contain rejected commits and, if double-checked, I'm for merging it into master.

comment:4 Changed 5 years ago by Olek Nowodziński

Milestone: CKEditor 4.4.2

comment:5 Changed 5 years ago by Piotrek Koszuliński

Thanks Olek and of course thanks danya_postfactum.

@danya_postfactum: do you agree with Olek's review (mainly - with the rejected comments)? We'll wait for your feedback before merging this branch into master, so we can bring back your commits which were rejected.

comment:6 Changed 5 years ago by Piotrek Koszuliński

Status: reviewreview_passed

Ok, time to merge this to master.

comment:7 Changed 5 years ago by Piotrek Koszuliński

Resolution: fixed
Status: review_passedclosed

Fixed on master with git:f5acc04. Huge thanks for the pull request!

Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy