Skip to content
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

Improve UI panel arrow design #2483

Closed
msamsel opened this issue Oct 15, 2018 · 6 comments
Closed

Improve UI panel arrow design #2483

msamsel opened this issue Oct 15, 2018 · 6 comments
Assignees
Labels
changelog:api A changelog entry should be put in the API section of the changelog. good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. status:confirmed An issue confirmed by the development team. target:major Any docs related issue that should be merged into a major branch. type:feature A feature request.
Milestone

Comments

@msamsel
Copy link
Contributor

msamsel commented Oct 15, 2018

Type of report

Feature request

Provide description of the new feature

Extracted from #2062 (comment)

Arrow next to UI panel button seems to be a little to close to icon. In case of emoji dropdown (maybe also in background) it's clearly visible. We should be able to somehow control position of this arrow or just improve its position to be better in such scenarios.
screen shot 2018-10-15 at 15 16 12
screen shot 2018-10-15 at 15 16 19

@msamsel msamsel added type:feature A feature request. status:confirmed An issue confirmed by the development team. labels Oct 15, 2018
@mlewand mlewand added good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. target:major Any docs related issue that should be merged into a major branch. labels Oct 15, 2018
@msamsel msamsel mentioned this issue Oct 16, 2018
2 tasks
@jacekbogdanski jacekbogdanski self-assigned this Oct 22, 2018
@jacekbogdanski
Copy link
Member

Arrow has already attached CSS class and so emoji button. I wonder if we really need special treatment for the case which could by already easily styled by CSS?

.cke_button__emojipanel .cke_button_arrow {
  margin-left: 5px;
}

WDYT @mlewand, @msamsel ?

@mlewand
Copy link
Contributor

mlewand commented Oct 22, 2018

I think the change should be applied to all icons that have this kind of arrow. Language/SCAYT dropdown also look kinda weird with the arrow so close to it.

However simply increasing causes our toolbar to go into 4 lines for the full preset, which is not an option. So we need to adjust that, maybe shaving some padding somewhere else?

@jacekbogdanski
Copy link
Member

I changed left margin from 1 to 2px. Unfortunately, the bigger margin will require cutting some other buttons to avoid line wrapping which don't seem good also. Take a look on screenshot please, especially @dkonopka :

image

@dkonopka
Copy link

☝️ Still, 2px looks too small spacing IMHO. How does it look with for example 4 pixels?

@jacekbogdanski
Copy link
Member

4 pixels look definitely better, although they wrap the first line of the toolbar which as @mlewand mentioned creates 4 lines for full CKEditor. I was wondering about changing the padding size of buttons but they are just perfect sized in my eye. Do you see some place where we can cut some pixels to fit full editor in 3 lines?

4px margin:
image

@jacekbogdanski
Copy link
Member

jacekbogdanski commented Oct 24, 2018

It seems like line wrapping is caused by container outside of the editor. It makes me wonder if we can change anything without sacrificing some space of existing toolbar elements. Any change resulting in bigger/smaller toolbar may break some users interfaces - even as small as 1px margin.

I think we may update a little our button element with class giving us information if the button is expandable and if so - shrink its left and right paddings to build a place for arrow margin. It's already possible with aria-haspopup="true" but I prefer to not rely CSS styles on aria labels. I will try this approach to see if it's working correctly.

@mlewand mlewand added this to the 4.11.0 milestone Oct 26, 2018
@mlewand mlewand added the changelog:api A changelog entry should be put in the API section of the changelog. label Oct 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:api A changelog entry should be put in the API section of the changelog. good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. status:confirmed An issue confirmed by the development team. target:major Any docs related issue that should be merged into a major branch. type:feature A feature request.
Projects
None yet
Development

No branches or pull requests

4 participants