Commit d772fbb0 authored by Gábor Hojtsy's avatar Gábor Hojtsy

Issue #3023801 by seanB, DamienMcKenna, lauriii, phenaproxima, Berdir, Pancho,...

Issue #3023801 by seanB, DamienMcKenna, lauriii, phenaproxima, Berdir, Pancho, marcoscano, Wim Leers, rainbreaw, larowlan, webchick, jrockowitz, andrewmacpherson, ckrina: Allow newly uploaded files to be deleted from the media library without saving them
parent bcbcbdca
<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16"><path fill="#ee0000" d="M3.51 13.925c.194.194.512.195.706.001l3.432-3.431c.194-.194.514-.194.708 0l3.432 3.431c.192.194.514.193.707-.001l1.405-1.417c.191-.195.189-.514-.002-.709l-3.397-3.4c-.192-.193-.192-.514-.002-.708l3.401-3.43c.189-.195.189-.515 0-.709l-1.407-1.418c-.195-.195-.513-.195-.707-.001l-3.43 3.431c-.195.194-.516.194-.708 0l-3.432-3.431c-.195-.195-.512-.194-.706.001l-1.407 1.417c-.194.195-.194.515 0 .71l3.403 3.429c.193.195.193.514-.001.708l-3.4 3.399c-.194.195-.195.516-.001.709l1.406 1.419z"/></svg>
......@@ -27,8 +27,10 @@
margin: 0;
}
/* @todo Use a class instead of the li element.
https://www.drupal.org/project/drupal/issues/3029227 */
/**
* @todo Use a class instead of the li element.
* https://www.drupal.org/project/drupal/issues/3029227
*/
.media-library-menu li {
display: block;
}
......@@ -84,17 +86,27 @@
}
/* Generic media add form styles. */
.media-library-add-form--without-input {
.media-library-add-form--without-input .form-item {
margin: 0 0 1em;
}
/**
* Remove outline from added media container.
*
* The added media container receives focus after adding new media, but since
* it is not an interactive element it does not need an outline.
*/
.media-library-add-form__added-media {
outline: none;
}
.media-library-add-form__input-wrapper {
padding: 16px;
border: 1px solid #bfbfbf;
border-radius: 2px;
background: #fcfcfa;
}
.media-library-add-form--without-input .form-item {
margin: 0 0 1em;
}
/* Style the media add upload form. */
.media-library-add-form--upload.media-library-add-form--without-input .form-item-upload {
margin-bottom: 0;
......@@ -105,12 +117,12 @@
}
/* Style the media add oEmbed form. */
.media-library-add-form-oembed-wrapper {
.media-library-add-form--oembed .media-library-add-form__input-wrapper {
display: flex;
}
@media screen and (max-width: 37.5em) {
.media-library-add-form-oembed-wrapper {
.media-library-add-form--oembed .media-library-add-form__input-wrapper {
display: block;
}
}
......@@ -123,8 +135,10 @@
width: 100%;
}
/* @todo Remove .button when styles are moved to the seven theme in
https://www.drupal.org/project/drupal/issues/2980769 */
/**
* @todo Remove .button when styles are moved to the seven theme in
* https://www.drupal.org/project/drupal/issues/2980769
*/
.button.media-library-add-form-oembed-submit {
align-self: center;
}
......@@ -152,8 +166,13 @@
margin: 0.75em 0;
}
/* Override the table display of the visually hidden labels so they won't take
up space. */
/**
* Override the table display of the visually hidden labels.
*
* The width, height and overflow properties in the styles for the
* .visually-hidden class do not work correctly if the element has a table
* display.
*/
.media-library-item label {
display: inline-block;
}
......@@ -166,8 +185,10 @@
justify-content: space-between;
}
/* @todo Remove order and reorder the views header and filters via a views
template in https://www.drupal.org/project/drupal/issues/3035994 */
/**
* @todo Remove order and reorder the views header and filters via a views
* template in https://www.drupal.org/project/drupal/issues/3035994
*/
.media-library-wrapper .view-header {
order: 2;
align-self: flex-end;
......@@ -178,21 +199,27 @@
text-align: left;
}
/* @todo Remove order and reorder the views header and filters via a views
template in https://www.drupal.org/project/drupal/issues/3035994 */
/**
* @todo Remove order and reorder the views header and filters via a views
* template in https://www.drupal.org/project/drupal/issues/3035994
*/
.media-library-wrapper .media-library-view .view-filters {
order: 1;
}
/* @todo Remove order and reorder the views header and filters via a views
template in https://www.drupal.org/project/drupal/issues/3035994 */
/**
* @todo Remove order and reorder the views header and filters via a views
* template in https://www.drupal.org/project/drupal/issues/3035994
*/
.media-library-wrapper .media-library-view .view-content {
flex: 0 0 100%;
order: 3;
}
/* @todo Remove order and reorder the views header and filters via a views
template in https://www.drupal.org/project/drupal/issues/3035994 */
/**
* @todo Remove order and reorder the views header and filters via a views
* template in https://www.drupal.org/project/drupal/issues/3035994
*/
.media-library-wrapper .media-library-view .pager {
order: 4;
}
......@@ -403,8 +430,10 @@
position: relative;
}
/* @todo Change to .media-library-open-button when styles are moved to the
seven theme in https://www.drupal.org/project/drupal/issues/2980769 */
/**
* @todo Change to .media-library-open-button when styles are moved to the
* seven theme in https://www.drupal.org/project/drupal/issues/2980769
*/
.button.media-library-open-button {
margin-left: 0; /* LTR */
}
......@@ -464,11 +493,19 @@
border-color: #40b6ff;
}
/* Style the wrappers around new media and files. */
/**
* Style the added media item container.
*
* The added media container receives screen reader focus since it is has the
* role 'listitem'. Since it is not an interactive element, it does not need
* an outline.
*/
.media-library-add-form__media {
position: relative;
display: flex;
padding: 20px 0 20px 0;
border-bottom: 1px solid #c0c0c0;
outline: none;
}
/* Do not show the top padding for the first item. */
......@@ -476,6 +513,16 @@
padding-top: 0;
}
/**
* Change the position of the remove button for the first item.
*
* The first item doesn't have a top padding, change the location of the remove
* button as well.
*/
.media-library-add-form__media:first-child .media-library-add-form__remove-button[type="submit"] {
top: 5px;
}
/* Do not show the bottom border and padding for the last item. */
.media-library-add-form__media:last-child {
padding-bottom: 0;
......@@ -504,6 +551,59 @@
margin-left: 20px;
}
/**
* @todo Remove [type="submit"] when styles are moved to the seven theme in
* https://www.drupal.org/project/drupal/issues/2980769
*/
.media-library-add-form__remove-button[type="submit"] {
position: absolute;
top: 25px;
right: 6px; /* LTR */
width: auto;
margin: 0;
padding: 2px 20px 2px 2px; /* LTR */
text-transform: lowercase;
color: transparent;
border: 0;
border-radius: 0;
background: transparent url(../../../misc/icons/787878/ex.svg) right 2px no-repeat; /* LTR */
font-weight: normal;
line-height: 16px;
}
[dir="rtl"] .media-library-add-form__remove-button[type="submit"] {
right: auto;
left: 13px;
padding: 2px 2px 2px 20px;
background-position: left 2px;
}
.media-library-add-form__remove-button:focus,
.media-library-add-form__remove-button.button:disabled,
.media-library-add-form__remove-button.button:disabled:active,
.media-library-add-form__remove-button.button:focus {
color: #787878;
border: 0;
background: transparent url(../../../misc/icons/787878/ex.svg) right 2px no-repeat; /* LTR */
}
[dir="rtl"] .media-library-add-form__remove-button:focus,
[dir="rtl"] .media-library-add-form__remove-button.button:disabled,
[dir="rtl"] .media-library-add-form__remove-button.button:disabled:active,
[dir="rtl"] .media-library-add-form__remove-button.button:focus {
background-position: left 2px;
}
.media-library-add-form__remove-button:hover,
.media-library-add-form__remove-button.button:hover {
color: #e00;
border: 0;
background: transparent url(../../../misc/icons/ee0000/ex.svg) right 2px no-repeat; /* LTR */
box-shadow: none;
}
[dir="rtl"] .media-library-add-form__remove-button:hover,
[dir="rtl"] .media-library-add-form__remove-button.button:hover {
background-position: left 2px;
}
/* @todo Remove or re-work in https://www.drupal.org/node/2985168 */
.media-library-widget .media-library-item__name a,
.media-library-view--widget .media-library-item__name a {
......
......@@ -122,8 +122,16 @@ protected function buildInputElement(array $form, FormStateInterface $form_state
$slots = $state->getAvailableSlots();
// Add a container to group the input elements for styling purposes.
$form['container'] = [
'#type' => 'container',
'#attributes' => [
'class' => ['media-library-add-form__input-wrapper'],
],
];
$process = (array) $this->elementInfo->getInfoProperty('managed_file', '#process', []);
$form['upload'] = [
$form['container']['upload'] = [
'#type' => 'managed_file',
'#title' => $this->formatPlural($slots, 'Add file', 'Add files'),
// @todo Move validation in https://www.drupal.org/node/2988215
......@@ -136,7 +144,7 @@ protected function buildInputElement(array $form, FormStateInterface $form_state
$file_upload_help = [
'#theme' => 'file_upload_help',
'#upload_validators' => $form['upload']['#upload_validators'],
'#upload_validators' => $form['container']['upload']['#upload_validators'],
'#cardinality' => $slots,
];
......@@ -145,7 +153,7 @@ protected function buildInputElement(array $form, FormStateInterface $form_state
// upload help in the same way, so any theming improvements made to file
// fields would also be applied to this upload field.
// @see \Drupal\file\Plugin\Field\FieldWidget\FileWidget::formElement()
$form['upload']['#description'] = $this->renderer->renderPlain($file_upload_help);
$form['container']['upload']['#description'] = $this->renderer->renderPlain($file_upload_help);
return $form;
}
......
......@@ -91,10 +91,11 @@ protected function buildInputElement(array $form, FormStateInterface $form_state
$media_type = $this->getMediaType($form_state);
$providers = $media_type->getSource()->getProviders();
// Add a container to group the input elements for styling purposes.
$form['container'] = [
'#type' => 'container',
'#attributes' => [
'class' => ['media-library-add-form-oembed-wrapper'],
'class' => ['media-library-add-form__input-wrapper'],
],
];
......
......@@ -597,6 +597,7 @@ public function testWidgetAnonymous() {
public function testWidgetUpload() {
$assert_session = $this->assertSession();
$page = $this->getSession()->getPage();
$driver = $this->getSession()->getDriver();
foreach ($this->getTestFiles('image') as $image) {
$extension = pathinfo($image->filename, PATHINFO_EXTENSION);
......@@ -735,11 +736,6 @@ public function testWidgetUpload() {
// Select a media item.
$page->find('css', '.media-library-view .js-click-to-select-checkbox input')->click();
$assert_session->pageTextContains('1 item selected');
// Multiple uploads should be allowed.
// @todo Add test when https://github.com/minkphp/Mink/issues/358 is closed
$this->assertTrue($assert_session->fieldExists('Add files')->hasAttribute('multiple'));
$page->attachFileToField('Add files', $this->container->get('file_system')->realpath($png_image->uri));
$assert_session->assertWaitOnAjaxRequest();
$page->fillField('Name', 'Unlimited Cardinality Image');
......@@ -822,6 +818,87 @@ public function testWidgetUpload() {
$assert_session->assertWaitOnAjaxRequest();
$assert_session->pageTextNotContains('Add or select media');
$assert_session->pageTextContains($jpg_image->filename);
// Assert removing an uploaded media item before save works as expected.
$assert_session->elementExists('css', '.media-library-open-button[name^="field_unlimited_media"]')->click();
$assert_session->assertWaitOnAjaxRequest();
$assert_session->pageTextContains('Add or select media');
$page->clickLink('Type Three');
$assert_session->assertWaitOnAjaxRequest();
$page->attachFileToField('Add files', $this->container->get('file_system')->realpath($png_image->uri));
$assert_session->assertWaitOnAjaxRequest();
// Assert the focus is shifted to the added media items.
$this->assertJsCondition('jQuery(".media-library-add-form__added-media").is(":focus")');
// Assert the media item fields are shown and the vertical tabs are no
// longer shown.
$assert_session->elementExists('css', '.media-library-add-form__fields');
$assert_session->elementNotExists('css', '.media-library-menu');
// Press the 'Remove button' and assert the user is sent back to the media
// library.
$assert_session->elementExists('css', '.media-library-add-form__remove-button')->click();
$assert_session->assertWaitOnAjaxRequest();
// Assert the remove message is shown.
$assert_session->pageTextContains("The media item $png_image->filename has been removed.");
// Assert the focus is shifted to the first tabbable element of the add
// form, which should be the source field.
$this->assertJsCondition('jQuery("#media-library-add-form-wrapper :tabbable").is(":focus")');
$assert_session->elementNotExists('css', '.media-library-add-form__fields');
$assert_session->elementExists('css', '.media-library-menu');
$page->find('css', '.ui-dialog-titlebar-close')->click();
// Assert uploading multiple files.
$assert_session->elementExists('css', '.media-library-open-button[name^="field_unlimited_media"]')->click();
$assert_session->assertWaitOnAjaxRequest();
$assert_session->pageTextContains('Add or select media');
$page->clickLink('Type Three');
$assert_session->assertWaitOnAjaxRequest();
$this->assertTrue($assert_session->fieldExists('Add files')->hasAttribute('multiple'));
// Create a list of new files to upload.
$filenames = [];
$remote_paths = [];
foreach (range(1, 3) as $i) {
$path = $file_system->copy($png_image->uri);
$filenames[] = $file_system->basename($path);
$remote_paths[] = $driver->uploadFileAndGetRemoteFilePath($file_system->realpath($path));
}
$page->findField('Add files')->setValue(implode("\n", $remote_paths));
$assert_session->assertWaitOnAjaxRequest();
// Assert the media item fields are shown and the vertical tabs are no
// longer shown.
$assert_session->elementExists('css', '.media-library-add-form__fields');
$assert_session->elementNotExists('css', '.media-library-menu');
// Assert all files have been added.
$assert_session->fieldValueEquals('media[0][fields][name][0][value]', $filenames[0]);
$assert_session->fieldValueEquals('media[1][fields][name][0][value]', $filenames[1]);
$assert_session->fieldValueEquals('media[2][fields][name][0][value]', $filenames[2]);
// Set alt texts for items 1 and 2, leave the alt text empty for item 3 to
// assert the field validation does not stop users from removing items.
$page->fillField('media[0][fields][field_media_test_image][0][alt]', $filenames[0]);
$page->fillField('media[1][fields][field_media_test_image][0][alt]', $filenames[1]);
// Remove the second file and assert the focus is shifted to the container
// of the next media item and field values are still correct.
$page->pressButton('media-1-remove-button');
$this->assertJsCondition('jQuery(".media-library-add-form__media[data-media-library-added-delta=2]").is(":focus")');
$assert_session->pageTextContains('The media item ' . $filenames[1] . ' has been removed.');
// The second media item should be removed (this has the delta 1 since we
// start counting from 0).
$assert_session->elementNotExists('css', '.media-library-add-form__media[data-media-library-added-delta=1]');
$media_item_one = $assert_session->elementExists('css', '.media-library-add-form__media[data-media-library-added-delta=0]');
$assert_session->fieldValueEquals('Name', $filenames[0], $media_item_one);
$assert_session->fieldValueEquals('Alternative text', $filenames[0], $media_item_one);
$media_item_three = $assert_session->elementExists('css', '.media-library-add-form__media[data-media-library-added-delta=2]');
$assert_session->fieldValueEquals('Name', $filenames[2], $media_item_three);
$assert_session->fieldValueEquals('Alternative text', '', $media_item_three);
// Remove the last file and assert the focus is shifted to the container
// of the first media item and field values are still correct.
$page->pressButton('media-2-remove-button');
$this->assertJsCondition('jQuery(".media-library-add-form__media[data-media-library-added-delta=0]").is(":focus")');
$assert_session->pageTextContains('The media item ' . $filenames[2] . ' has been removed.');
$assert_session->elementNotExists('css', '.media-library-add-form__media[data-media-library-added-delta=1]');
$assert_session->elementNotExists('css', '.media-library-add-form__media[data-media-library-added-delta=2]');
$media_item_one = $assert_session->elementExists('css', '.media-library-add-form__media[data-media-library-added-delta=0]');
$assert_session->fieldValueEquals('Name', $filenames[0], $media_item_one);
$assert_session->fieldValueEquals('Alternative text', $filenames[0], $media_item_one);
}
/**
......@@ -921,6 +998,33 @@ public function testWidgetOEmbed() {
$assert_session->assertWaitOnAjaxRequest();
$assert_session->pageTextNotContains('Add or select media');
$assert_session->pageTextContains('Custom video title');
// Assert removing an added oEmbed media item before save works as expected.
$assert_session->elementExists('css', '.media-library-open-button[name^="field_unlimited_media"]')->click();
$assert_session->assertWaitOnAjaxRequest();
$assert_session->pageTextContains('Add or select media');
$page->clickLink('Type Five');
$assert_session->assertWaitOnAjaxRequest();
$page->fillField('Add Type Five via URL', $video_url);
$page->pressButton('Add');
$assert_session->assertWaitOnAjaxRequest();
// Assert the focus is shifted to the added media items.
$this->assertJsCondition('jQuery(".media-library-add-form__added-media").is(":focus")');
// Assert the media item fields are shown and the vertical tabs are no
// longer shown.
$assert_session->elementExists('css', '.media-library-add-form__fields');
$assert_session->elementNotExists('css', '.media-library-menu');
// Press the 'Remove button' and assert the user is sent back to the media
// library.
$assert_session->elementExists('css', '.media-library-add-form__remove-button')->click();
$assert_session->assertWaitOnAjaxRequest();
// Assert the remove message is shown.
$assert_session->pageTextContains("The media item $video_title has been removed.");
// Assert the focus is shifted to the first tabbable element of the add
// form, which should be the source field.
$this->assertJsCondition('jQuery("#media-library-add-form-wrapper :tabbable").is(":focus")');
$assert_session->elementNotExists('css', '.media-library-add-form__fields');
$assert_session->elementExists('css', '.media-library-menu');
}
}
......@@ -87,7 +87,7 @@ public function testMediaTypeAddForm() {
]));
// Assert the media library UI only contains the add form for the image
// media type.
$this->assertSame('managed_file', $this->buildLibraryUi('image')['content']['form']['upload']['#type']);
$this->assertSame('managed_file', $this->buildLibraryUi('image')['content']['form']['container']['upload']['#type']);
$this->assertEmpty($this->buildLibraryUi('remote_video')['content']['form']);
// Create a user that has access to create both media types.
......@@ -97,7 +97,7 @@ public function testMediaTypeAddForm() {
]));
// Assert the media library UI only contains the add form for both media
// types.
$this->assertSame('managed_file', $this->buildLibraryUi('image')['content']['form']['upload']['#type']);
$this->assertSame('managed_file', $this->buildLibraryUi('image')['content']['form']['container']['upload']['#type']);
$this->assertSame('url', $this->buildLibraryUi('remote_video')['content']['form']['container']['url']['#type']);
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment