Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

MAGE-6758: Link Position Setter Doesn't Work in Magento #37

Closed
choukalos opened this Issue · 7 comments

4 participants

@choukalos

There are two different ways to add navigation link in magento:

Mage_Page_Block_Template_Links::addLink method:
and
Mage_Page_Block_Template_Links::addLinkBlock method

When we try to add link using addLink method - it will sort array by position: ksort($this->_links);,
but when we add link by addLinkBlock($blockName) method it will add link by square bracket syntax: $this->_links[$this->_getNewPosition((int)$block->getPosition())] = $block;
In this case we produce wrong links order, because php add element at the end of array (index value doesn't make sense)
See php manual: http://www.php.net/manual/en/language.types.array.php#language.types.array.syntax.modifying

Possible solution:

Easy way:
Create public method for sorting links array then use it in addLink method. Add array sorting for addLinkBlock method.

Unix way:

  • We don't need to reorder link every time when we add new one (see _getNewPosition method). We could store link prosition in varien object and reorder them just before we try to get links in template (see getLinks method).
  • addLinkBlock method doesn't check object interface to be compatible with links, so we need to check instanse or interface of the object that we try to add as link
  • _beforeToHtml method set array pointer to the end of array - this can case misunderstanding in template when we get links by getLinks() method

Also try this flow -
Preconditions
1. Customer is registered
2. Order is created
Steps
1. Login to Frontend
2. Open "My Orders" tab
3. Open "View Order" page

Actual Result: "Items Ordered" grid show before order information

Expected result: "Items Ordered" grid show below order information

Bugathon PM - Ashish Kumar

@benmarks benmarks was assigned
@benmarks
Collaborator

Suggested flow does not demonstrate the issue:
Screen Shot 2013-03-09 at 9 34 41 PM

@benmarks
Collaborator

Two commits:

  • c24d284 - Provides a resolution to the missing ksort() for link blocks
  • 93afdc7 - reset()s the array pointer on the _links block in _beforeToHtml()
@artembychkov

Benjamin,

I was not able to look up the commits that you referenced. Could you please provide the direct links to them.

Thanks in advance!

@benmarks
Collaborator

@artembychkov

I can submit a PR if that will help. Let me know. Thanks!

@artembychkov

@benmarks

Please do. Thank you!

@benmarks
Collaborator

PR submitted: #275

@piotrekkaminski

This issue was merged and should be available as part of CE 1.8.1 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.