diem-project / diem

Diem CMF CMS for symfony 1.4
http://diem-project.org/
MIT License
184 stars 85 forks source link

dmMenu logic for creating span entries is wrong #359

Closed antitoxic closed 13 years ago

antitoxic commented 13 years ago

When Diem generates menu entries it checks whether the current page id is equal to entrie's link id. But consider the example that the current page is list and you are on page 2. In this case surely the link must be active and not span element but it is.

My proposal is to add one more check. A function that checks whether the current request URI (without the root part) is equal to page slug. Thus the system is able to understand if the content is different - paged or filtered.

The following is the required method to do so with all names I can logically think of: class dmFrontLinkTagPage ....

  public function isExplicit()
  public function isDistinct()
  public function isDefinite()
  public function isPrecise()
  public function isOriginal()
  public function isCurrentConclusive()
  public function isCurrentDistinct()
  public function isCurrentExact()unambiguous,
  public function isCurrentUnambiguous(),
  public function isCurrentLiteral(),
  public function isCurrentStrict(),
  public function isCurrentStrict()
  {
    $reqContext = $this->requestContext;
    $relativeToRootRequestUri = str_replace($reqContext['uri_prefix'].$reqContext['prefix'], '', $reqContext['request_uri']);
    return ltrim($relativeToRootRequestUri,'/') == $this->currentPage->getSlug();
  }

Then in prepareAttributesForHtml should be checked along with the check for creating spans: protected function prepareAttributesForHtml(array $attributes) { ... // current page if($this->isCurrent() && empty($attributes['anchor'])) { $attributes['class'][] = $attributes['current_class'];

          if($attributes['current_span'] && $this->isCurrentStrict())
          {
             $attributes['tag'] = 'span';
          }
        }
...

If any of Diem staff agree to the logic and pick a name I will push it to my fork so that you can apply the changes.

stephaneerard commented 13 years ago

I'll have to dig it, but please make your fork, push and I'll test your commits.

Thank you !

Regards,

antitoxic commented 13 years ago

Closed by antitoxic/diem@f3158c6dd5879646f6f04ebb634bbeb66437f9b4

stephaneerard commented 13 years ago

It has been merged by a pull request, no ? I guess it might be better to put pull requests/commits urls from diem-project/diem repository ;)

Thank you !