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

Happychat: Fix sidebar sizing issues in Editor and other places #12270

Merged
merged 3 commits into from
Mar 20, 2017

Conversation

jasmussen
Copy link
Member

@jasmussen jasmussen commented Mar 17, 2017

This changes the panel mode to be the base style, and the sidebar mode to be CSS that overrides the panel mode. This allows us to enable the panel mode on larger breakpoints, without having to resort to ugly duplicate CSS hacks.

As part of this, I removed a bunch of old hacks, and simply invoked the panel mode instead. This includes the editor, the reader, and looking at a single theme. It would be nice if we could have the sidebar in the reader, but zooming into a single reader post makes the sidebar overlap the full bleed card.

This also fixes an issue in Happychat with the new editor.

Because it's major CSS surgery, would appreciate a good review, many eyes, and tests in various browsers. Thank you.

Steps to test:

  • Load up this branch, or the calypso.live version.
  • Open up a chat.
  • Visit as many sections of Calypso as you care to.

Please try the above in as many browsers as you have the energy for, and verify that things look right. And by "looking right" I mean no obvious layout issues. The chat will be in panel mode on some pages, that is intended.

This changes the panel mode to be the base style, and the sidebar mode to be CSS that overrides the panel mode. This allows us to enable the panel mode on larger breakpoints, without having to resort to ugly duplicate CSS hacks.

As part of this, I removed a bunch of old hacks, and simply invoked the panel mode instead. This includes the editor, the reader, and looking at a single theme. It would be nice if we could have the sidebar in the reader, but zooming into a single reader post makes the sidebar overlap the full bleed card.

This also fixes an issue in Happychat with the new editor.

Because it's major CSS surgery, would appreciate a good review, many eyes, and tests in various browsers. Thank you.
@jasmussen jasmussen added [Feature] Post/Page Editor The editor for editing posts and pages. Happychat [Feature] Reader The reader site on Calypso. [Type] Bug labels Mar 17, 2017
@jasmussen jasmussen self-assigned this Mar 17, 2017
@matticbot
Copy link
Contributor

@jasmussen jasmussen requested a review from beaucollins March 17, 2017 12:20
@jasmussen
Copy link
Member Author

Tested in Chrome and Firefox on the Mac.

I can't seem to load http://calypso.localhost:3000/help at all on Safari (which seems unrelated to the changes in this PR) — anyone else?

@jasmussen
Copy link
Member Author

Tested it in Safari via calypso.live. Works fine.

@oandregal
Copy link
Contributor

Tested in chrome on Linux (Ubuntu).

My understanding is that sidebar mode means happychat in the right sidebar taking up all the vertical space and panel mode means happychat in a floating small window in the right.

Tried several sections (help, customizer, editor, pages, etc) and viewports. All look good. In a big viewport, all sections show the sidebar mode except in the editor, where I could only get the panel mode.

@jasmussen
Copy link
Member Author

jasmussen commented Mar 17, 2017

My understanding is that sidebar mode means happychat in the right sidebar taking up all the vertical space and panel mode means happychat in a floating small window in the right.

Correct!

Tried several sections (help, customizer, editor, pages, etc) and viewports. All look good. In a big viewport, all sections show the sidebar mode except in the editor, where I could only get the panel mode.

Sounds like it works as intended, then. Thanks for testing.

Also tested in Android Chrome, worked fine.

Only Windows browsers, mobile Safari, and perhaps a code review remaining, looks like!

@kriskarkoski
Copy link
Contributor

Should the saved message be appearing over the post settings? This is in a narrowed Chrome window:

screen shot 2017-03-17 at 10 33 03 am

And here it is with Happychat opened:

screen shot 2017-03-17 at 10 33 14 am

@jasmussen
Copy link
Member Author

First off, do you have a very large screen Kris?

If yes, then great catch! Seems like an editor issue, can you add to #11996

@oandregal
Copy link
Contributor

Did a bit more testing in the editor with the post settings sidebar and the chat panel as well. This is how it looks the transition between medium and large viewports.

medium viewport at maximum size:

vp-medium-max

big viewport at minimum size:

vp-large-min

Could repro the same issue on chrome and FF, on Linux. If either the post settings or the chat pannel is not activated, the editor has a more smooth transition between viewports.

@kriskarkoski
Copy link
Contributor

I am not sure if this was a result of these changes, but occasionally the cursor and first line drop behind the toolbar. I haven't been able to reproduce it consistently, but it seems to happen most often after toggling the post settings to hidden:

screen shot 2017-03-17 at 12 29 20 pm

@jasmussen
Copy link
Member Author

Thanks for the thorough test results, some interesting findings here. I'll take a look early next week!

@timmyc
Copy link
Contributor

timmyc commented Mar 17, 2017

@kriskarkoski that likely could be from #12078 - I thought I had fixed all of those lingering issues, but will play around with that in this branch too. By chance do you remember the screen size when you saw this?

@kriskarkoski
Copy link
Contributor

@timmyc It should be right around 425-430px wide, I am not sure on the height but it was pretty tall as it was on by Thunderbolt monitor as I was testing changing the width of a bigger window at the time. If you can't repro please let me know and I will be happy to record some testing and note the screen size once I see it.

@jasmussen
Copy link
Member Author

The weird viewport sizing issues that were reported here #12270 (comment) should be fixed now:

screen shot 2017-03-20 at 09 27 42

@timmyc I'm going to try and get this merged in, hope that's okay. I'll take a look at the footer issues as well as some other bugs in a separate branch, feel free to join in on that one.

@jasmussen jasmussen added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 20, 2017
Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

This is testing out great for me 👍

Tested on Chrome, Firefox and Safari Mac.

With the new editor, when we have HappyChat open, we can't access some options on the sidebar ("More Options" hidden a bit behind the happy chat panel) but it seems unrelated to this PR

@jasmussen jasmussen merged commit 1da55fb into master Mar 20, 2017
@jasmussen jasmussen deleted the fix/editor-happychat-issues branch March 20, 2017 09:09
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 20, 2017
jhnstn pushed a commit that referenced this pull request Mar 21, 2017
* Fix issue with word count overlap.

* Rewrite a fair bit of the panel/sidebar code.

This changes the panel mode to be the base style, and the sidebar mode to be CSS that overrides the panel mode. This allows us to enable the panel mode on larger breakpoints, without having to resort to ugly duplicate CSS hacks.

As part of this, I removed a bunch of old hacks, and simply invoked the panel mode instead. This includes the editor, the reader, and looking at a single theme. It would be nice if we could have the sidebar in the reader, but zooming into a single reader post makes the sidebar overlap the full bleed card.

This also fixes an issue in Happychat with the new editor.

Because it's major CSS surgery, would appreciate a good review, many eyes, and tests in various browsers. Thank you.

* Fix sizing issues with the visual editor bar.

Props Andrés.
@matticbot matticbot added the [Size] XL Probably needs to be broken down into multiple smaller issues label Mar 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Post/Page Editor The editor for editing posts and pages. [Feature] Reader The reader site on Calypso. Happychat [Size] XL Probably needs to be broken down into multiple smaller issues [Type] Bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants