-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Media: Add a settings dialog to the Media Library toolbar. #12932
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
f1bc9ac
a98eea4
9251623
b0bb18a
2b58600
a10841e
3aac1c6
d280e56
837d3b4
ee61aef
5421a07
d22db3c
382cdfe
e70b2c5
f407c53
49ba43d
961d5b7
b67cddb
dd4fe91
eb9a031
739c979
2b23278
c8617bc
2ca898f
431a1b9
cf528d3
93997ba
8562d2a
cd6074a
155999b
fe09da6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,8 @@ var View = wp.media.View, | |
| l10n = wp.media.view.l10n, | ||
| $ = jQuery, | ||
| AttachmentsBrowser, | ||
| infiniteScrolling = wp.media.view.settings.infiniteScrolling, | ||
| settings = wp.media.view.settings, | ||
| librarySettings = wp.media.view.settings.librarySettings, | ||
| __ = wp.i18n.__, | ||
| sprintf = wp.i18n.sprintf; | ||
|
|
||
|
|
@@ -34,6 +35,8 @@ AttachmentsBrowser = View.extend(/** @lends wp.media.view.AttachmentsBrowser.pro | |
| className: 'attachments-browser', | ||
|
|
||
| initialize: function() { | ||
| var infiniteScrolling = !! settings.infiniteScrolling; | ||
|
|
||
| _.defaults( this.options, { | ||
| filters: false, | ||
| search: true, | ||
|
|
@@ -45,6 +48,7 @@ AttachmentsBrowser = View.extend(/** @lends wp.media.view.AttachmentsBrowser.pro | |
|
|
||
| this.controller.on( 'toggle:upload:attachment', this.toggleUploader, this ); | ||
| this.controller.on( 'edit:selection', this.editSelection ); | ||
| this.controller.on( 'library:infinite-scrolling', this.setInfiniteScrolling, this ); | ||
|
|
||
| // In the Media Library, the sidebar is used to display errors before the attachments grid. | ||
| if ( this.options.sidebar && 'errors' === this.options.sidebar ) { | ||
|
|
@@ -111,6 +115,42 @@ AttachmentsBrowser = View.extend(/** @lends wp.media.view.AttachmentsBrowser.pro | |
| this.collection.on( 'attachments:received', this.announceSearchResults, this ); | ||
| }, | ||
|
|
||
| /** | ||
| * Switches between infinite scrolling and the Load more button in place. | ||
| * | ||
| * The Load more view is created on demand and then kept, hidden by the | ||
| * `has-load-more` class, so that switching back and forth is cheap. | ||
| * | ||
| * @since 7.1.0 | ||
| * | ||
| * @param {boolean} enabled Whether to load more attachments on scroll. | ||
| * | ||
| * @return {void} | ||
| */ | ||
| setInfiniteScrolling: function( enabled ) { | ||
| if ( enabled === Boolean( this.attachments.options.infiniteScrolling ) ) { | ||
| return; | ||
| } | ||
|
|
||
| this.$el.toggleClass( 'has-load-more', ! enabled ); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might need to cover a few more items/classes based on this logic, like
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 8562d2a |
||
| this.attachments.setInfiniteScrolling( enabled ); | ||
|
|
||
| if ( enabled ) { | ||
| this.collection.off( 'add remove reset', this.updateLoadMoreView, this ); | ||
| this.$el.removeClass( 'more-loaded' ); | ||
| this.$el.find( '.found-media' ).removeClass( 'found-media' ); | ||
| this.$el.find( '.new-media' ).removeClass( 'new-media' ); | ||
| return; | ||
| } | ||
|
|
||
| if ( ! this.loadMoreWrapper ) { | ||
| this.createLoadMoreView(); | ||
| } | ||
|
|
||
| this.collection.on( 'add remove reset', this.updateLoadMoreView, this ); | ||
| this.updateLoadMoreView(); | ||
| }, | ||
|
|
||
| /** | ||
| * Updates the `wp.a11y.speak()` ARIA live region with a message to communicate | ||
| * the number of search results to screen reader users. This function is | ||
|
|
@@ -125,7 +165,7 @@ AttachmentsBrowser = View.extend(/** @lends wp.media.view.AttachmentsBrowser.pro | |
| /* translators: Accessibility text. %d: Number of attachments found in a search. */ | ||
| mediaFoundHasMoreResultsMessage = __( 'Number of media items displayed: %d. Click load more for more results.' ); | ||
|
|
||
| if ( infiniteScrolling ) { | ||
| if ( this.attachments.options.infiniteScrolling ) { | ||
| /* translators: Accessibility text. %d: Number of attachments found in a search. */ | ||
| mediaFoundHasMoreResultsMessage = __( 'Number of media items displayed: %d. Scroll the page for more results.' ); | ||
| } | ||
|
|
@@ -393,6 +433,14 @@ AttachmentsBrowser = View.extend(/** @lends wp.media.view.AttachmentsBrowser.pro | |
| model: this.collection.props, | ||
| priority: 60 | ||
| }).render() ); | ||
|
|
||
| // Only for users allowed to save the preference, see `wp_enqueue_media()`. | ||
| if ( librarySettings ) { | ||
| this.toolbar.set( 'librarySettings', new wp.media.view.LibrarySettings({ | ||
| controller: this.controller, | ||
| priority: 70 | ||
| }).render() ); | ||
| } | ||
| } | ||
|
|
||
| if ( this.options.dragInfo ) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,194 @@ | ||
| var View = wp.media.View, | ||
| settings = wp.media.view.settings, | ||
| $ = jQuery, | ||
| __ = wp.i18n.__, | ||
| LibrarySettings; | ||
|
|
||
| /** | ||
| * wp.media.view.LibrarySettings | ||
| * | ||
| * A toolbar control opening a modal dialog with the personal options for the | ||
| * Media Library. | ||
| * | ||
| * @since 7.1.0 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Considering the 7.1 branch is only for bug fixes at this point, I doubt if this will make it. Should we consider it for 7.2?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will need to be agreed upon and decided among the release leads. |
||
| * | ||
| * @memberOf wp.media.view | ||
| * | ||
| * @class | ||
| * @augments wp.media.View | ||
| * @augments wp.Backbone.View | ||
| * @augments Backbone.View | ||
| */ | ||
| LibrarySettings = View.extend(/** @lends wp.media.view.LibrarySettings.prototype */{ | ||
| tagName: 'button', | ||
| className: 'button button-compact media-library-settings__toggle', | ||
| template: wp.template( 'media-library-settings-toggle' ), | ||
|
|
||
| attributes: { | ||
| type: 'button', | ||
| 'aria-haspopup': 'dialog' | ||
| }, | ||
|
|
||
| events: { | ||
| 'click': 'open' | ||
| }, | ||
|
|
||
| /** | ||
| * Whether a request is in progress. | ||
| * | ||
| * @type {boolean} | ||
| */ | ||
| isSaving: false, | ||
|
|
||
| initialize: function() { | ||
| // Several media frames can be attached at once, so IDs are per instance. | ||
| this.uid = _.uniqueId( 'media-library-settings-' ); | ||
| }, | ||
|
|
||
| prepare: function() { | ||
| return { | ||
| titleId: this.uid + '-title', | ||
| infiniteScrollingId: this.uid + '-infinite-scrolling' | ||
| }; | ||
| }, | ||
|
|
||
| /** | ||
| * Removes the dialog along with the view. | ||
| * | ||
| * @return {wp.media.view.LibrarySettings} Returns itself to allow chaining. | ||
| */ | ||
| dispose: function() { | ||
| if ( this.dialog ) { | ||
| $( this.dialog ).remove(); | ||
| } | ||
|
|
||
| return View.prototype.dispose.apply( this, arguments ); | ||
| }, | ||
|
|
||
| /** | ||
| * Inserts the dialog into the frame. | ||
| * | ||
| * @return {void} | ||
| */ | ||
| createDialog: function() { | ||
| var $dialog = $( wp.template( 'media-library-settings-dialog' )( this.prepare() ) ); | ||
|
|
||
| this.controller.$el.append( $dialog ); | ||
|
|
||
| this.dialog = $dialog[0]; | ||
| this.status = $dialog.find( '.media-library-settings__status' )[0]; | ||
| this.checkbox = $dialog.find( '.media-library-settings__checkbox' )[0]; | ||
|
|
||
| $dialog.on( 'change', '.media-library-settings__checkbox', _.bind( this.updateInfiniteScrolling, this ) ); | ||
|
|
||
| /* | ||
| * In the media modal, `wp.media.view.Modal` closes on Escape and | ||
| * `wp.media.view.FocusManager` constrains Tab. Neither must run while the | ||
| * dialog is open: it handles both itself, and the rest of the modal is inert. | ||
| */ | ||
| $dialog.on( 'keydown', function( event ) { | ||
| event.stopPropagation(); | ||
| } ); | ||
| }, | ||
|
|
||
| /** | ||
| * Opens the dialog. | ||
| * | ||
| * @return {void} | ||
| */ | ||
| open: function() { | ||
| if ( ! this.dialog ) { | ||
| this.createDialog(); | ||
| } | ||
|
|
||
| if ( ! this.isSaving ) { | ||
| this.checkbox.checked = !! settings.librarySettings.infiniteScrolling; | ||
| this.setStatus( '' ); | ||
| } | ||
|
|
||
| this.dialog.showModal(); | ||
| }, | ||
|
|
||
| /** | ||
| * Whether a `media_library_infinite_scrolling` filter callback overrides the | ||
| * personal option. | ||
| * | ||
| * @type {boolean} | ||
| */ | ||
| isFiltered: !! settings.librarySettings && !! settings.librarySettings.isFiltered, | ||
|
|
||
| /** | ||
| * Saves the "Infinite scrolling" personal option for the current user. | ||
| * | ||
| * @return {void} | ||
| */ | ||
| updateInfiniteScrolling: function() { | ||
| if ( ! this.isSaving ) { | ||
| this.save(); | ||
| } | ||
| }, | ||
|
|
||
| /** | ||
| * Sends the state of the checkbox to the server, then whatever it was toggled | ||
| * to in the meantime. | ||
| * | ||
| * @return {void} | ||
| */ | ||
| save: function() { | ||
| var view = this, | ||
| enabled = this.checkbox.checked; | ||
|
|
||
| this.isSaving = true; | ||
|
|
||
| this.setStatus( __( 'Saving…' ) ); | ||
|
|
||
| wp.ajax.post( 'set-media-library-settings', { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Multiple concurrent saves can result in saving the wrong value. Might need some throttling or disabling if a request is in progress here.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 93997ba |
||
| _ajax_nonce: settings.librarySettings.nonce, | ||
| infinite_scrolling: enabled ? 'true' : 'false' | ||
| } ).done( function() { | ||
| view.isSaving = false; | ||
|
|
||
| settings.librarySettings.infiniteScrolling = enabled ? 1 : 0; | ||
|
|
||
| if ( enabled !== view.checkbox.checked ) { | ||
| view.save(); | ||
| return; | ||
| } | ||
|
|
||
| /* | ||
| * A filter callback takes precedence over the preference, so the Media | ||
| * Library keeps the filtered behavior. Otherwise the browser this toggle | ||
| * belongs to is updated without a reload. | ||
| */ | ||
| if ( ! view.isFiltered ) { | ||
| settings.infiniteScrolling = enabled ? 1 : 0; | ||
|
|
||
| view.controller.trigger( 'library:infinite-scrolling', enabled ); | ||
| } | ||
|
|
||
| view.setStatus( enabled ? | ||
| __( 'Infinite scrolling is on.' ) : | ||
| __( 'Infinite scrolling is off.' ) | ||
| ); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this yet another place that might be out of sync with
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am considering this, and I have two proposals.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To me, the first option is more clear. |
||
| } ).fail( function( response ) { | ||
| view.isSaving = false; | ||
|
|
||
| // Put the checkbox back in sync with the stored value. | ||
| view.checkbox.checked = !! settings.librarySettings.infiniteScrolling; | ||
|
|
||
| view.setStatus( ( response && response.message ) || __( 'The setting could not be saved.' ) ); | ||
| } ); | ||
| }, | ||
|
|
||
| /** | ||
| * Updates the message below the controls. | ||
| * | ||
| * @param {string} message The message to display. An empty string clears it. | ||
| * @return {void} | ||
| */ | ||
| setStatus: function( message ) { | ||
| this.status.textContent = message; | ||
| } | ||
| }); | ||
|
|
||
| module.exports = LibrarySettings; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noting that
this.scroll()never checksthis.options.infiniteScrollingso if there's already a queued throttled callback or pendingcollection.more(), they might callscroll()again, which will run unchecked. We might want to add athis.options.infiniteScrollingcheck toscroll()itself.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 431a1b9