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

Fix behavior of update button #83

Merged
merged 2 commits into from Aug 27, 2016

Conversation

Projects
None yet
2 participants
@westonruter
Contributor

westonruter commented Aug 24, 2016

@PatelUtkarsh I need your help to fix some apparent regressions that were introduced with scheduling. Namely there is logic currently in populateSetting that seems like it should be refactored and moved to addButtons.

See #81 for details on the bug needing to be fixed.

Fixes #81

@westonruter westonruter added this to the 0.5.2 milestone Aug 24, 2016

} else {
save.html( component.data.i18n.updateButton );
save.text( component.data.i18n.saveButton );

This comment has been minimized.

@westonruter

westonruter Aug 24, 2016

Contributor

I think all of this logic related to the buttons should be moved to addButton.

This comment has been minimized.

@PatelUtkarsh

PatelUtkarsh Aug 24, 2016

Collaborator

We could move this logic to addButton but we also want to tie this logic on date input change so I kept it separate. But yes I see the advantage of keeping all button logic in addButton.
Maybe we should create a event for schedule and bind this button logic in addButton with that event? (using wp.customize.state.create)

This comment has been minimized.

@westonruter

westonruter Aug 24, 2016

Contributor

Yes, creating more states would be good and then listening for changes to those states in a centralized location I think is the best architecture.

@PatelUtkarsh

This comment has been minimized.

Collaborator

PatelUtkarsh commented Aug 25, 2016

@westonruter I've refactored code as you suggested in #83 (comment) as part of PR #76 - e27ed55
Let me know if it requires any changes.

Bug #81 is also fixed in 3e26906

@westonruter westonruter changed the title from [WIP] Fix behavior of update button to Fix behavior of update button Aug 27, 2016

@westonruter westonruter merged commit 2a1ce07 into develop Aug 27, 2016

1 check passed

continuous-integration/travis-ci/push The Travis CI build passed
Details

@westonruter westonruter deleted the bugfix/update-button branch Aug 27, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment