Uploaded image for project: 'phpBB3'
  1. phpBB3
  2. PHPBB3-9479

Empty paragraphs for vertical spacing are rather old-fashioned

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.0.x
    • Fix Version/s: 3.1.0-b1
    • Component/s: Other
    • Labels:
      None

      Description

      This is strictly a cosmetic issue with the XHTML, and not user-visible.

      In the world of reliably functional CSS, using empty paragraphs for vertical spacing is rather old-fashioned and obsolete. I suggest that the following be replaced with appropriate CSS margins:

      Edit: I've now got what I think is a better version of this - see next post.
      I'm leaving this version in so you can see the alternates.

      styles/prosilver/template/jumpbox.html:

      <!-- IF S_VIEWTOPIC -->
              <p></p><p><a href="{U_VIEW_FORUM}" class="left-box {S_CONTENT_FLOW_BEGIN}" accesskey="r">{L_RETURN_TO} {FORUM_NAME}</a></p>
      <!-- ELSEIF S_VIEWFORUM -->
              <p></p><p><a href="{U_INDEX}" class="left-box {S_CONTENT_FLOW_BEGIN}" accesskey="r">{L_RETURN_TO} {L_INDEX}</a></p>
      <!-- ELSEIF SEARCH_TOPIC -->
              <p></p><p><a class="left-box {S_CONTENT_FLOW_BEGIN}" href="{U_SEARCH_TOPIC}" accesskey="r">{L_RETURN_TO}: {SEARCH_TOPIC}</a></p>
      <!-- ELSEIF S_SEARCH_ACTION -->
              <p></p><p><a class="left-box {S_CONTENT_FLOW_BEGIN}" href="{U_SEARCH}" title="{L_SEARCH_ADV}" accesskey="r">{L_RETURN_TO_SEARCH_ADV}</a></p>
      <!-- ENDIF -->

      Replace with:

      <!-- IF S_VIEWTOPIC -->
              <div class="jumpbox-return-to"><a href="{U_VIEW_FORUM}" class="left-box {S_CONTENT_FLOW_BEGIN}" accesskey="r">{L_RETURN_TO} {FORUM_NAME}</a></div>
      <!-- ELSEIF S_VIEWFORUM -->
              <div class="jumpbox-return-to"><a href="{U_INDEX}" class="left-box {S_CONTENT_FLOW_BEGIN}" accesskey="r">{L_RETURN_TO} {L_INDEX}</a></div>
      <!-- ELSEIF SEARCH_TOPIC -->
              <div class="jumpbox-return-to"><a class="left-box {S_CONTENT_FLOW_BEGIN}" href="{U_SEARCH_TOPIC}" accesskey="r">{L_RETURN_TO}: {SEARCH_TOPIC}</a></div>
      <!-- ELSEIF S_SEARCH_ACTION -->
              <div class="jumpbox-return-to"><a class="left-box {S_CONTENT_FLOW_BEGIN}" href="{U_SEARCH}" title="{L_SEARCH_ADV}" accesskey="r">{L_RETURN_TO_SEARCH_ADV}</a></div>
      <!-- ENDIF -->

      It's largely a matter of personal taste, but I also prefer using <div> for non-paragraph markup blocks. By all means stay with <p> (just strip the leading <p></p> from each line), if I've missed some subtlety there.

      Add to styles/prosilver/theme/common.css:

      .jumpbox-return-to {
      	margin-top: 2em;
      }

      A quick grep through prosilver/template/*.html didn't throw up any other instances of <p> being used in this manner, but my search was not exhaustive.

      The similar "<br /><br />" near the bottom of jumpbox.html can also be removed and replaced with CSS:

      <!-- ELSE -->
      	<br /><br />

      Find in styles/prosilver/theme/common.css:

      #page-footer {
              clear: both;
      }

      Change to:

      #page-footer {
              clear: both;
              padding-top: 5px;
      }

      Find in styles/prosilver/theme/forms.css:

      fieldset.jumpbox {
              text-align: right;
              margin-top: 15px;
              height: 2.5em;
      }

      Change to:

      fieldset.jumpbox {
              text-align: right;
              margin-top: 15px;
      }

        Activity

        Hide
        paul.j.murphy paul.j.murphy added a comment -

        After a bit of puzzling over odd box margin behaviour with styles/prosilver/template/jumpbox.html on /search.php?search_id=active_topics, I've come up with a change which seems to make the margins behave more sensibly in Firefox and Safari (I don't use Windows, so no testing done on MSIE). The "float:left;" from class="left-box" on the RETURN_TO links was causing the bottom margin to fail to properly collide with the nav bar below it. So, with a bit of surgery to move all the floating to the right, here's what I've got.

        jumpbox.html (entire file, since it's small):

        <!-- IF S_DISPLAY_JUMPBOX -->
        	<form method="post" id="jumpbox" action="{S_JUMPBOX_ACTION}" onsubmit="if(document.jumpbox.f.value == -1){return false;}">
         
        	<!-- IF $CUSTOM_FIELDSET_CLASS -->
        		<fieldset class="{$CUSTOM_FIELDSET_CLASS}">
        	<!-- ELSE -->
        		<fieldset class="jumpbox">
        	<!-- ENDIF -->
        			<label for="f" accesskey="j"><!-- IF S_IN_MCP and S_MERGE_SELECT -->{L_SELECT_TOPICS_FROM}<!-- ELSEIF S_IN_MCP -->{L_MODERATE_FORUM}<!-- ELSE -->{L_JUMP_TO}<!-- ENDIF -->:</label>
        			<select name="f" id="f" onchange="if(this.options[this.selectedIndex].value != -1){ document.forms['jumpbox'].submit() }">
        			<!-- BEGIN jumpbox_forums -->
        				<!-- IF jumpbox_forums.S_FORUM_COUNT == 1 --><option value="-1">------------------</option><!-- ENDIF -->
        				<option value="{jumpbox_forums.FORUM_ID}"{jumpbox_forums.SELECTED}><!-- BEGIN level -->&nbsp; &nbsp;<!-- END level -->{jumpbox_forums.FORUM_NAME}</option>
        			<!-- END jumpbox_forums -->
        			</select>
        			<input type="submit" value="{L_GO}" class="button2" />
        		</fieldset>
        	</form>
        <!-- ENDIF -->
         
        <!-- IF S_VIEWTOPIC -->
        	<div class="jumpbox-return-to"><a class="{S_CONTENT_FLOW_BEGIN}" href="{U_VIEW_FORUM}" accesskey="r">{L_RETURN_TO} {FORUM_NAME}</a></div>
        <!-- ELSEIF S_VIEWFORUM -->
        	<div class="jumpbox-return-to"><a class="{S_CONTENT_FLOW_BEGIN}" href="{U_INDEX}" accesskey="r">{L_RETURN_TO} {L_INDEX}</a></div>
        <!-- ELSEIF SEARCH_TOPIC -->
        	<div class="jumpbox-return-to"><a class="{S_CONTENT_FLOW_BEGIN}" href="{U_SEARCH_TOPIC}" accesskey="r">{L_RETURN_TO}: {SEARCH_TOPIC}</a></div>
        <!-- ELSEIF S_SEARCH_ACTION -->
        	<div class="jumpbox-return-to"><a class="{S_CONTENT_FLOW_BEGIN}" href="{U_SEARCH}" title="{L_SEARCH_ADV}" accesskey="r">{L_RETURN_TO_SEARCH_ADV}</a></div>
        <!-- ENDIF -->

        I took the liberty of re-ordering the attribites on <a> so they are in the same order across the 4 lines.

        Add to common.css:

        .jumpbox-return-to {
                margin-top: 17px; // 17px derived from fieldset.jumpbox: 15px margin + 1px border + 1px paddiing
        }

        Change forums.css:

        /* Jumpbox */
        fieldset.jumpbox {
                text-align: right;
                margin-top: 15px;
                height: 2.5em;
        }

        to:

        /* Jumpbox */
        fieldset.jumpbox {
                float: right;
                margin-top: 15px;
                margin-bottom: 5px;
        }

        Show
        paul.j.murphy paul.j.murphy added a comment - After a bit of puzzling over odd box margin behaviour with styles/prosilver/template/jumpbox.html on /search.php?search_id=active_topics, I've come up with a change which seems to make the margins behave more sensibly in Firefox and Safari (I don't use Windows, so no testing done on MSIE). The "float:left;" from class="left-box" on the RETURN_TO links was causing the bottom margin to fail to properly collide with the nav bar below it. So, with a bit of surgery to move all the floating to the right, here's what I've got. jumpbox.html (entire file, since it's small): <!-- IF S_DISPLAY_JUMPBOX --> <form method="post" id="jumpbox" action="{S_JUMPBOX_ACTION}" onsubmit="if(document.jumpbox.f.value == -1){return false;}">   <!-- IF $CUSTOM_FIELDSET_CLASS --> <fieldset class="{$CUSTOM_FIELDSET_CLASS}"> <!-- ELSE --> <fieldset class="jumpbox"> <!-- ENDIF --> <label for="f" accesskey="j"><!-- IF S_IN_MCP and S_MERGE_SELECT -->{L_SELECT_TOPICS_FROM}<!-- ELSEIF S_IN_MCP -->{L_MODERATE_FORUM}<!-- ELSE -->{L_JUMP_TO}<!-- ENDIF -->:</label> <select name="f" id="f" onchange="if(this.options[this.selectedIndex].value != -1){ document.forms['jumpbox'].submit() }"> <!-- BEGIN jumpbox_forums --> <!-- IF jumpbox_forums.S_FORUM_COUNT == 1 --><option value="-1">------------------</option><!-- ENDIF --> <option value="{jumpbox_forums.FORUM_ID}"{jumpbox_forums.SELECTED}><!-- BEGIN level -->&nbsp; &nbsp;<!-- END level -->{jumpbox_forums.FORUM_NAME}</option> <!-- END jumpbox_forums --> </select> <input type="submit" value="{L_GO}" class="button2" /> </fieldset> </form> <!-- ENDIF -->   <!-- IF S_VIEWTOPIC --> <div class="jumpbox-return-to"><a class="{S_CONTENT_FLOW_BEGIN}" href="{U_VIEW_FORUM}" accesskey="r">{L_RETURN_TO} {FORUM_NAME}</a></div> <!-- ELSEIF S_VIEWFORUM --> <div class="jumpbox-return-to"><a class="{S_CONTENT_FLOW_BEGIN}" href="{U_INDEX}" accesskey="r">{L_RETURN_TO} {L_INDEX}</a></div> <!-- ELSEIF SEARCH_TOPIC --> <div class="jumpbox-return-to"><a class="{S_CONTENT_FLOW_BEGIN}" href="{U_SEARCH_TOPIC}" accesskey="r">{L_RETURN_TO}: {SEARCH_TOPIC}</a></div> <!-- ELSEIF S_SEARCH_ACTION --> <div class="jumpbox-return-to"><a class="{S_CONTENT_FLOW_BEGIN}" href="{U_SEARCH}" title="{L_SEARCH_ADV}" accesskey="r">{L_RETURN_TO_SEARCH_ADV}</a></div> <!-- ENDIF --> I took the liberty of re-ordering the attribites on <a> so they are in the same order across the 4 lines. Add to common.css: .jumpbox-return-to { margin-top: 17px; // 17px derived from fieldset.jumpbox: 15px margin + 1px border + 1px paddiing } Change forums.css: /* Jumpbox */ fieldset.jumpbox { text-align: right; margin-top: 15px; height: 2.5em; } to: /* Jumpbox */ fieldset.jumpbox { float: right; margin-top: 15px; margin-bottom: 5px; }
        Hide
        nickvergessen Joas Schilling added a comment -

        We do not use empty <p></p> anymore.

        Show
        nickvergessen Joas Schilling added a comment - We do not use empty <p></p> anymore.

          People

          • Assignee:
            nickvergessen Joas Schilling
            Reporter:
            paul.j.murphy paul.j.murphy
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development