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

Reader: Fix full post view closing when any click on popover menu #1208

Merged
merged 2 commits into from
Dec 4, 2015

Conversation

dongseok0
Copy link
Contributor

Full post view is wrapped by dialog and any click on popover menu
fires clickOutside event that closes the full post view. It's because
popover view is attached on body tag.

To fix it, prevent default action of click outside event for full post view.
#686

Full post view is wrapped by dialog and any click on popover menu
fires clickOutside event that closes the full post view. It's because
popover view is attached on body tag.

To fix it, prevent default action of click outside event for full post view.
@lancewillett lancewillett added [Feature] Reader The reader site on Calypso. [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 3, 2015
@blowery
Copy link
Contributor

blowery commented Dec 3, 2015

Hi @dongseok0, thanks for the PR! Works great, but I noticed that it exposes a new problem. If you view a post from a blog you own in the reader, an Edit Post action appears. If you pick that action from the menu, we move over to the editor, but the Full Post Dialog doesn't close. It appears we can just add an exit route handler:

diff --git a/client/reader/index.js b/client/reader/index.js
index 600dfef..bc0bc9b 100644
--- a/client/reader/index.js
+++ b/client/reader/index.js
@@ -39,11 +39,11 @@ module.exports = function() {
                page.exit( '/read/blog/feed/:feed_id', controller.resetTitle );

                page( '/read/post/feed/:feed/:post', setLastRoute, controller.feedPost );
-               page.exit( '/read/post/feed/:feed/:post', controller.resetTitle );
+               page.exit( '/read/post/feed/:feed/:post', controller.resetTitle, controller.removePost );

                page( '/read/blog/id/:blog_id', saveLastRoute, controller.removePost, controller.sidebar, controller.blogListing );
                page( '/read/post/id/:blog/:post', setLastRoute, controller.blogPost );
-               page.exit( '/read/post/id/:blog/:post', controller.resetTitle );
+               page.exit( '/read/post/id/:blog/:post', controller.resetTitle, controller.removePost );

                page( '/tag/:tag', saveLastRoute, controller.removePost, controller.sidebar, controller.tagListing );
        }

Could you add that to the PR? Should be good to go.

@blowery blowery added [Status] Needs Author Reply and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 3, 2015
Add exit route handler to close FullPostDialog
@dongseok0
Copy link
Contributor Author

Added exit route handlers! 😄

@blowery
Copy link
Contributor

blowery commented Dec 4, 2015

Thank you!

screenshot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Reader The reader site on Calypso. OSS Citizen
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants