Uploaded image for project: 'Wicket'
  1. Wicket
  2. WICKET-5531

Create new placeholder tag to indicate where header contributions should appear

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 6.14.0
    • Fix Version/s: 7.0.0-M1, 6.15.0
    • Component/s: wicket
    • Labels:
      None

      Description

      It would be easier to manage exactly where header contributions are positioned in the head of the document if there was a placeholder tag which could be used to indicate the position. Much in the same way as wicket:child indicates where the child markup is positioned.

      This idea from Martin-G.

      See discussion at http://mail-archives.apache.org/mod_mbox/wicket-dev/201304.mbox/%3CCAMomwMqRds-PhWkR_%3DR1x-4HH9sSW41H1nARacU%2BN15uJHh9SA%40mail.gmail.com%3E

      1. wicket-5531-quickstart-2.tar.gz
        19 kB
        Jesse Long
      2. wicket-5531.tar.gz
        18 kB
        Jesse Long

        Issue Links

          Activity

          Hide
          mgrigorov Martin Grigorov added a comment -

          A working draft of the new feature can be found at branch 5531-wicket-header-items (created from wicket-6.x).
          TODO:

          • add tests
          • document it
          Show
          mgrigorov Martin Grigorov added a comment - A working draft of the new feature can be found at branch 5531-wicket-header-items (created from wicket-6.x). TODO: add tests document it
          Hide
          mgrigorov Martin Grigorov added a comment -

          Please give it a try with 6.15.0-SNAPSHOT if you can so we can clean any inefficiencies before 6.15.0 release.

          Show
          mgrigorov Martin Grigorov added a comment - Please give it a try with 6.15.0-SNAPSHOT if you can so we can clean any inefficiencies before 6.15.0 release.
          Hide
          jesselong Jesse Long added a comment -

          Hi Martin,

          I am testing with 6.15.0-SNAPSHOT, but it seems that the header items all appear twice:

          <head>
          <meta charset="utf-8" />
          <title>Apache Wicket Quickstart</title>

          <!-- THIS SHOULD BEFORE AFTER ALL HEADER ITEMS -->

          <!-- PRIORITY HEADER ITEM FROM PAGE CODE -->
          <!-- HEAD FROM PANEL MARKUP -->
          <!-- HEADER FROM PANEL CODE -->
          <!-- HEADER ITEM FROM SUB PAGE MARKUP -->
          <!-- HEADER ITEM FROM PAGE CODE -->

          <!-- THIS SHOULD APPEAR AFTER ALL HEADER ITEMS -->
          <head><!-- PRIORITY HEADER ITEM FROM PAGE CODE -->
          <!-- HEAD FROM PANEL MARKUP -->
          <!-- HEADER FROM PANEL CODE -->
          <!-- HEADER ITEM FROM SUB PAGE MARKUP -->
          <!-- HEADER ITEM FROM PAGE CODE --></head></head>

          Show
          jesselong Jesse Long added a comment - Hi Martin, I am testing with 6.15.0-SNAPSHOT, but it seems that the header items all appear twice: <head> <meta charset="utf-8" /> <title>Apache Wicket Quickstart</title> <!-- THIS SHOULD BEFORE AFTER ALL HEADER ITEMS --> <!-- PRIORITY HEADER ITEM FROM PAGE CODE --> <!-- HEAD FROM PANEL MARKUP --> <!-- HEADER FROM PANEL CODE --> <!-- HEADER ITEM FROM SUB PAGE MARKUP --> <!-- HEADER ITEM FROM PAGE CODE --> <!-- THIS SHOULD APPEAR AFTER ALL HEADER ITEMS --> <head><!-- PRIORITY HEADER ITEM FROM PAGE CODE --> <!-- HEAD FROM PANEL MARKUP --> <!-- HEADER FROM PANEL CODE --> <!-- HEADER ITEM FROM SUB PAGE MARKUP --> <!-- HEADER ITEM FROM PAGE CODE --></head></head>
          Hide
          jesselong Jesse Long added a comment -

          quickstart which shows all header items duplicated.

          Show
          jesselong Jesse Long added a comment - quickstart which shows all header items duplicated.
          Hide
          mgrigorov Martin Grigorov added a comment -

          Thanks for noticing this !
          I have improved it.

          Show
          mgrigorov Martin Grigorov added a comment - Thanks for noticing this ! I have improved it.
          Hide
          jesselong Jesse Long added a comment -

          Thanks, working 100% now. Thank you for implementing this Martin, very nice feature.

          Show
          jesselong Jesse Long added a comment - Thanks, working 100% now. Thank you for implementing this Martin, very nice feature.
          Hide
          jesselong Jesse Long added a comment - - edited

          Hi Martin,

          Some further testing shows that this is not always working. My original quickstart worked fine, but this new quickstart (wicket-5531-quickstart-2.tar.gz) does not work. Please click on the link on the home page to see the error.

          I have tried to debug it, but I'm not sure my solution is correct. Definitely not elegant.

          I was able to fix it by overriding getMarkup() in HtmlHeaderItemsContainer as show below. The code is very similar to what happens in HtmlHeaderContainer, but I think HtmlHeaderItemsContainer should probably just look for the first <wicket:header-items> component tag and forget about anything else. Yes? I dont know what ComponentTag.getMarkupClass() is all about so I'm hesitant to start passing judgement on whether or not HtmlHeaderContainer.getMarkup() is correct. Please be so kind as to make this into an elegant solution.

              @Override
              public IMarkupFragment getMarkup()
              {
          	if (getParent() == null){
          	    throw new WicketRuntimeException(
          		    "Bug: The Wicket internal instance of HtmlHeaderContainer is not connected to a parent");
          	}
          
          	// Get the page markup
          	IMarkupFragment markup = getPage().getMarkup();
          	if (markup == null){
          	    throw new MarkupException("Unable to get page markup: " + getPage().toString());
          	}
          
          	// Find the markup fragment
          	MarkupStream stream = new MarkupStream(markup);
          	IMarkupFragment headerMarkup = null;
          	while (stream.skipUntil(ComponentTag.class) && (headerMarkup == null)){
          	    ComponentTag tag = stream.getTag();
          	    if (tag.isOpen() || tag.isOpenClose()){
          		if (tag instanceof WicketTag){
          		    WicketTag wtag = (WicketTag)tag;
          #################################################
          Look here
          #################################################
          		    if (wtag.isHeaderItemsTag()){
          			headerMarkup = stream.getMarkupFragment();
          			break;
          		    }
          #################################################
          /Look here
          #################################################
          		}
          	    }
          
          	    stream.next();
          	}
          
          	setMarkup(headerMarkup);
          	return headerMarkup;
              }
          
          Show
          jesselong Jesse Long added a comment - - edited Hi Martin, Some further testing shows that this is not always working. My original quickstart worked fine, but this new quickstart (wicket-5531-quickstart-2.tar.gz) does not work. Please click on the link on the home page to see the error. I have tried to debug it, but I'm not sure my solution is correct. Definitely not elegant. I was able to fix it by overriding getMarkup() in HtmlHeaderItemsContainer as show below. The code is very similar to what happens in HtmlHeaderContainer, but I think HtmlHeaderItemsContainer should probably just look for the first <wicket:header-items> component tag and forget about anything else. Yes? I dont know what ComponentTag.getMarkupClass() is all about so I'm hesitant to start passing judgement on whether or not HtmlHeaderContainer.getMarkup() is correct. Please be so kind as to make this into an elegant solution. @Override public IMarkupFragment getMarkup() { if (getParent() == null){ throw new WicketRuntimeException( "Bug: The Wicket internal instance of HtmlHeaderContainer is not connected to a parent"); } // Get the page markup IMarkupFragment markup = getPage().getMarkup(); if (markup == null){ throw new MarkupException("Unable to get page markup: " + getPage().toString()); } // Find the markup fragment MarkupStream stream = new MarkupStream(markup); IMarkupFragment headerMarkup = null; while (stream.skipUntil(ComponentTag.class) && (headerMarkup == null)){ ComponentTag tag = stream.getTag(); if (tag.isOpen() || tag.isOpenClose()){ if (tag instanceof WicketTag){ WicketTag wtag = (WicketTag)tag; ################################################# Look here ################################################# if (wtag.isHeaderItemsTag()){ headerMarkup = stream.getMarkupFragment(); break; } ################################################# /Look here ################################################# } } stream.next(); } setMarkup(headerMarkup); return headerMarkup; }
          Hide
          mgrigorov Martin Grigorov added a comment -

          Thanks, Jesse!
          I've removed the check for the tag's markupClass. I cannot find a reason why this check was/is needed before. All seems to work fine without this check, and all tests pass.

          Show
          mgrigorov Martin Grigorov added a comment - Thanks, Jesse! I've removed the check for the tag's markupClass. I cannot find a reason why this check was/is needed before. All seems to work fine without this check, and all tests pass.

            People

            • Assignee:
              mgrigorov Martin Grigorov
              Reporter:
              jesselong Jesse Long
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development