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

Update dependencies and tag for 5.2 compatibility #28

Merged
merged 5 commits into from
Jul 29, 2019

Conversation

adamsilverstein
Copy link

Description of the Change

  • Update dependencies in package.json and composer.json to current versions.
  • Fix some PHP notices w/PHP 5.6 and fatals with PHP 7.2/3.

Fixes #19

Verification Process

  • Test with WordPress 5.2 and WordPress trunk.

Test with PHP 5.6 & 7.2.

Verify works as expected:

  • Print Issues: create, add articles, change article statuses, Export for InDesign: all and checked.
  • Issue Statuses, Publications, Article Statuses: create and use.
  • Test Gutenberg compatibility: no changes are made to the post editor with this plugin so far. The "Print" custom post type added by the plugin opens in the classic edit environment because show_in_rest is not set (Gutenberg requires REST API access to use a CPT). Everything works as expected in my testing.

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Applicable Issues

@adamsilverstein adamsilverstein changed the title Feature/updates for 5 2 Update dependencies and tag for 5.2 compatibility Jul 22, 2019
@jeffpaul jeffpaul added this to the 1.1.0 milestone Jul 22, 2019
Copy link
Member

@jeffpaul jeffpaul left a comment

Choose a reason for hiding this comment

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

looks good to me, defer to @helen on final approval

#minor-publishing-actions,.misc-pub-section.misc-pub-post-status,.misc-pub-section.misc-pub-visibility{display:none}#submitdiv{border:none;display:block;margin-bottom:20px;background:0 0!important}#submitdiv .hndle{background:#fff;border:1px solid #e5e5e5}#minor-publishing{background:#fff;border:1px solid #e5e5e5;border-top:0}#major-publishing-actions{padding:0;border:none;margin-top:20px;background:#fff}#major-publishing-actions .hndle{border:1px solid #e5e5e5}#publishing-actions{padding:8px 12px;border:1px solid #e5e5e5;border-top:0}#timestamp hide,.timestamp-wrap hide{display:none}.button-secondary.dashicons{width:50px}.edw-loading{width:20px;display:inline-block;vertical-align:middle}.postbox[id^=pi-sections-template]{display:none}#pi-section-add-info{display:none}.pi-error-msg{display:none}#pi-section-name{width:232px}#pi-section-add-confirm,#pi-section-name{display:inline-block;vertical-align:middle}.postbox span[contenteditable]{cursor:text;display:inline-block;vertical-align:middle;margin-right:10px}.postbox span[contenteditable].locked{cursor:wait;color:grey}.pi-article-add-info{display:none}.pi-article-results{display:block;clear:both;display:none}.pi-article-title{width:182px}.pi-article-search,.pi-article-title{display:inline-block;vertical-align:middle}select.pi-article-dropdown{max-width:300px}.pi-articles tfoot{display:none}.pi-article-remove,.pi-article-view{visibility:hidden;margin-left:10px}.pi-articles .column-title:hover .pi-article-remove,.pi-articles .column-title:hover .pi-article-view{visibility:visible}.pi-article-remove{color:#a00}.pi-article-remove:hover{color:red}.column-bulk-article-status input.cb-select-all-article-status{margin-left:0}th#bulk-article-status{width:2.2em}th.column-article-status{width:15%}th.column-date{width:auto!important}th.column-title{width:28%}.article-export-buttons{clear:left;margin-top:15px}
Copy link
Contributor

Choose a reason for hiding this comment

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

I wanted to manually review what's changed here, I think it's fine (although a little odd IMO) but just wanted to note things in case it becomes a problem later:

  1. Rather than grouping a bunch of selectors for the same usage of a given property (border, display), the new build seems to prefer to keep all properties for a selector together.
  2. In #publishing-actions, it goes from padding: 8px 9pt to padding: 8px 12px. The Sass file has 8px 12px so that's a fix IMO.
  3. .pi-article-results now keeps both display: block and display: none as in the source - I actually cannot figure out where pi-article-results is used or generated at the moment, so that probably deserves investigation at some point. It may be that it's a default-inline element being made a block element and hidden/shown using display so it kind of makes sense, but it's also weird and would have been broken before. We may not need this ruleset at all.

@adamsilverstein adamsilverstein merged commit 9fd7e5a into develop Jul 29, 2019
@adamsilverstein adamsilverstein deleted the feature/updates-for-5-2 branch July 29, 2019 22:20
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.

Test against WordPress 5.2
3 participants