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

Menus: fix randomly closing item edit view #1234

Merged
merged 1 commit into from
Dec 3, 2015
Merged

Conversation

seear
Copy link
Contributor

@seear seear commented Dec 3, 2015

Observing sites in menus/main.jsx forces a re-render whenever sites changes.
When this happens, the edit-tracking lock object is created anew, causing any currently
open item edit box to close.

This change removes the use of the lock lib entirely, using state to track
the currently edited item.

To Reproduce

  1. Go to https://wordpress.com/menus and select a site
  2. Click on the edit icon of an existing menu item

screen shot 2015-12-03 at 12 44 43

3) The edit view will open for the item 4) Wait a while, and the edit view will close on its own

To Test
With this PR, all menus behaviour should be the same, except for the edit view closing on its own

@seear seear added Menus [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 3, 2015
@seear seear self-assigned this Dec 3, 2015
@ehg
Copy link
Member

ehg commented Dec 3, 2015

The code LGTM, and I've tested moving/saving/adding/editing. All works. Perhaps we should loop @mcsf in to have a quick look?

@mcsf
Copy link
Member

mcsf commented Dec 3, 2015

This looks good so far. Killing Lock feels good. :)

My only remark would be that we're using the term editItem in the incoming changes, while the thing actually stored is just the ID.

@seear
Copy link
Contributor Author

seear commented Dec 3, 2015

My only remark would be that we're using the term editItem in the incoming changes, while the thing actually stored is just the ID.

Yeah, good point. Fixed in d95056c

Observing `sites` in menus/main.jsx forces a re-render whenever `sites` changes.
When this happens, the edit-tracking `lock` object is created anew, causing any currently
open item edit box to close.

This change removes the use of the lock lib entirely, using state to track
the currently edited item.
seear added a commit that referenced this pull request Dec 3, 2015
Menus: fix randomly closing item edit view
@seear seear merged commit a08faa8 into master Dec 3, 2015
@scruffian scruffian removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Dec 3, 2015
@seear seear deleted the fix/menu-edit-rerender branch December 3, 2015 18:26
@lancewillett
Copy link
Contributor

Thanks for fixing this! Was annoying me earlier this week and I came to report it to find a merged PR. Boom. 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants