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

t/416: Improved responsiveness of the forms in narrow viewports #1299

Merged
merged 3 commits into from
Oct 26, 2018
Merged

Conversation

oleq
Copy link
Member

@oleq oleq commented Oct 12, 2018

Suggested merge commit message (convention)

Feature: Improved responsiveness of the forms in narrow viewports (see #416).


Additional information

Things to decide

  • Is the media query breakpoint correct https://github.com/ckeditor/ckeditor5-theme-lark/compare/t/ckeditor5/416?expand=1#diff-5ac1a3c4aac63c20ab773daf9fbe1983R7? I picked it at random from the net (it's the screen size of some iPhone :P) but maybe we can do better than that.
    • Also... should we use max-device-width instead?
  • If you browse the branches you'll notice that a lot of code repeats because the UI is almost the same in each case: [ input/url preview button, save button, cancel button ]. Maybe we could give such a structure a class name, implement it somewhere, and make the existing code and this new responsive approach shorter. This could be a follow–up.

image

image

image

image

@oleq oleq requested review from Reinmar and dkonopka October 12, 2018 14:03
@oleq
Copy link
Member Author

oleq commented Oct 12, 2018

P.S. I consider this situation and an edge case and a sacrifice for the sake of an easy solution

image

@dkonopka
Copy link
Contributor

First of all with this PR I would recommend to add viewport settings in manual tests:
<meta name="viewport" content="width=device-width, initial-scale=1">.

https://github.com/ckeditor/ckeditor5-dev/blob/master/packages/ckeditor5-dev-tests/lib/utils/manual-tests/template.html#L4

@dkonopka
Copy link
Contributor

If you browse the branches you'll notice that a lot of code repeats because the UI is almost the same in each case: [ input/url preview button, save button, cancel button ]

I agree this is not perfect solution, but ATM we can live with it I guess.

Anyway, I was thinking about something like mixin for buttons in the forms (_rwd-form-buttons) to not duplicate code, but as you said it can be a follow-up.

@dkonopka
Copy link
Contributor

And my last thought, if we are using --ck-spacing-standard for labels, then it would be great to use it for buttons too (instead of --ck-spacing-tiny).

ATM (left), proposal (right)

spacings

@oleq
Copy link
Member Author

oleq commented Oct 25, 2018

That's a good idea @dkonopka! It's way easier to tap. I added it in ckeditor/ckeditor5-theme-lark@33c974c.

Oh, BTW, can you finish the review, please?

Copy link
Contributor

@dkonopka dkonopka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@dkonopka dkonopka merged commit 1a0e50b into master Oct 26, 2018
dkonopka added a commit that referenced this pull request Oct 26, 2018
Feature: Improved responsiveness of the forms in narrow viewports (see #416).
@oleq oleq deleted the t/416 branch October 26, 2018 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants