There should be no empty headers like <h2 id="views-ajax-title"></h2> which appears in admin/structure/views/view/test (after you create the View "test")

I pulled out more html than was needed, but thought it might be useful for context.

<div id="views-ajax-popup" class="form-wrapper"><h2 id="views-ajax-title"></h2><div id="views-ajax-body" class="form-wrapper"><div class="message">Click on an item to edit that item's details.</div></div></div><input type="hidden" name="form_build_id" value="form-kyGXhR5hTAenhx3lMUNa51Hel59bJfbe0MURpzc91m4" /><input type="hidden" name="form_token" value="DOCx1Kp8QMrJ67rDfMhwJM28NPaekHB_iYs_Z53hoRA" /><input type="hidden" name="form_id" value="view_edit_form" /><div class="form-actions form-wrapper" id="edit-actions"><input type="submit" id="edit-actions-submit" name="op" value="Save" class="button button-primary form-submit" /><input type="submit" id="edit-actions-cancel" name="op" value="Cancel" class="button form-submit" /></div></div></form></div><div id="views-preview-wrapper" class="views-admin clearfix"><form class="view-preview-form view-form" action="/admin/structure/views/view/teest/preview/page_1" method="post" id="views-ui-preview-form" accept-charset="UTF-8"><div><div class="form-item form-type-checkbox form-item-live-preview">
 <input type="checkbox" id="edit-displays-live-preview" name="live_preview" value="1" checked="checked" class="form-checkbox" /> <label class="option" for="edit-displays-live-preview">Auto preview</label>
</div>
<div class="form-item form-type-textfield form-item-view-args">
 <label for="preview-args">Preview with contextual filters:</label> <input aria-describedby="preview-args--description" type="text" id="preview-args" name="view_args" value="" size="60" maxlength="128" class="form-text" />

Discovered with the WAVE Toolbar.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mgifford’s picture

Older, closed, related issue for context #901128: Add additional headers to Views UI

dawehner’s picture

Issue tags: +Novice, +VDC

.

terrill’s picture

This issue applies to all semantic elements, not just headings.

For example, there's an empty article element nested inside <div class="meta submitted">, which is included as part of each article header on the home page:

<article about="/user/4" typeof="sioc:UserAccount" class="profile" data-edit-entity="user/4"></article>

JAWS 14 actually announces these elements as "Empty article", which is sure to confuse users. I think a best practice would be to add semantic elements dynamically, only if they're known to contain content. If they aren't used, don't clutter the page with them.

bowersox’s picture

Please review this suggested approach. I want to make sure we get the right game-plan before someone starts coding.

Possible Approach:

  • Change the H2 element into a DIV element. This will always be on the page, but will start out empty. This would be a change to core/modules/views_ui/lib/Drupal/views_ui/ViewEditFormController.php
  • When the popup dialog opens up and the title gets put into place with JS, that is when we actually put the H2 tag in there too. Inside the DIV we would put H2, title text, and /H2. This would be a change in core/modules/views_ui/js/ajax.js.
  • Do testing of the layout and the JS to make sure it can open and close this popup dialog and it shows up with the same visual look as currently.

Is that the right approach?

bowersox’s picture

Issue tags: +TwinCities
bowersox’s picture

Status: Active » Needs review
FileSize
1.82 KB

Please review this patch.

It makes that basic changes suggested, and updates one CSS style to keep the visual appearance the same.

mparker17’s picture

Status: Needs review » Reviewed & tested by the community

Code looks good and the views AJAX dialogs work correctly!

alexpott’s picture

Status: Reviewed & tested by the community » Needs review
+++ b/core/modules/views_ui/css/views_ui.admin.theme.cssundefined
@@ -896,9 +896,10 @@ ul#views-display-menu-tabs li.add ul.action-list li{
+  margin: 0;

How come we've had to add this? Obviously to keep the visual style the same but it'd be nice to know why.

bowersox’s picture

@alexpott, the line you referred to doesn't have a plus next to it in the patch, it's just context. The patch only adds this margin:

-.views-ui-dialog #views-ajax-title {
+.views-ui-dialog #views-ajax-title h2 {
   font-size: 15px;
   padding: 8px 13px;
+  margin: 0;
 }

And why is this margin needed? Because by default, core/themes/seven/style.css line 35 gives all H2 tags a margin: 10px 0. We don't want any margin around the #views-ajax-title.

In the current behavior (before this patch), the unwanted margin is zero'd out by core/modules/views_ui/css/views_ui.admin.css line 278:

.views-ui-dialog #views-ajax-title, .views-ui-dialog #views-ajax-body {
  margin: 0;
  padding: 0;
}

(This code applies because the H2 tag *is* the #views-ajax-title.)

In the proposed new behavior (after the patch), the H2 tag becomes nested inside of div#views-ajax-title. So that CSS above does not apply, and the H2 inherits the default 10px margin from style.css. So that's why the patch includes a margin:0.

I hope that explanation helps.

alexpott’s picture

So do we still need the css for .views-ui-dialog #views-ajax-title? Or can we remove this?

bowersox’s picture

@alexpott, I don't see any extra CSS cruft that we can get rid of. The patch changes the style that had applied to .views-ui-dialog #views-ajax-title and has that apply to .views-ui-dialog #views-ajax-title h2. So everything left in the CSS is needed. I looked at it again tonight, and there's no extra CSS cruft that I can see.

alexpott’s picture

So why can't

.views-ui-dialog #views-ajax-title, .views-ui-dialog #views-ajax-body {
  margin: 0;
  padding: 0;
}

become

.views-ui-dialog #views-ajax-body {
  margin: 0;
  padding: 0;
}
bowersox’s picture

@alexpott, you're right. That works fine. I've updated the patch.

Please review and test this new patch. The only change from last time is the one extraneous CSS selector removed.

dawehner’s picture

Adding some tags.

dawehner’s picture

Issue tags: +Novice, +TwinCities

yeah.

dobrzyns’s picture

FileSize
221.56 KB

I've created a test view on a clean Drupal install and am unable to find the empty h2. I did not open any settings popups. Working backwards, the closest I can get is the two hidden form inputs:

<input type="hidden" name="form_build_id" value="form-kyGXhR5hTAenhx3lMUNa51Hel59bJfbe0MURpzc91m4" /><input type="hidden" name="form_token" value="DOCx1Kp8QMrJ67rDfMhwJM28NPaekHB_iYs_Z53hoRA" /><input type="hidden" name="form_id" value="view_edit_form" />

There should then be three closing

tags, but I am seeing more than three closing
tags.
views-ui-empty-h2.png

What steps were taken to produce the empty h2?

dawehner’s picture

You have look at the preview of the view.

bowersox’s picture

Here are the steps to reproduce the bug which is fixed by the patch in comment 13:

1. Log on as an administrator (with ability to edit views).
2. Navigate to Adminstration > Structure > Views.
3. Choose any view and click "edit". For example, my URL is /admin/structure/views/view/content .
4. Now examine the page source. You will find an empty H2 like this:
<h2 id="views-ajax-title"></h2>

With the patch applied, this empty H2 is not there any more. If you confirm that, then this patch is ready to be set to RTBC.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs manual testing
FileSize
28.83 KB

I have seen it in the html.

h2.png

There simply can't be enough divs.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.