nobuti / Codeigniter-breadcrumbs

Small breadcrumb library for CodeIgniter
70 stars 68 forks source link

Unshift don't work properly #8

Closed tjapro closed 9 years ago

tjapro commented 9 years ago

The function unshift() in library don't work properly, because miss the following line:

$href = site_url($href);
nobuti commented 9 years ago

The push function already includes the site_url helper call, why do you want to re-do this again? I don't understand that logic.

tjapro commented 9 years ago

But isn't the push function. Is in the unshift function. The push function isn't called when I call the unshift function. An example:

nobuti commented 9 years ago

But before to unshift, you need to push. It's in the push where we are applying the site_url helper. This is the use case scenario.

tjapro commented 9 years ago

Ok. I did what you say me, but the results are these:

Controller file:
$this -> breadcrumbs -> push ('Home', '/');
$this -> breadcrumbs -> unshift('Home', '/');
$data['breadcrumbs'] = $this -> breadcrumbs -> show();

Result in browser:
<section class="col-md-4 col-lg-3">
            <nav>
                <ol class="breadcrumb"><li class="active">Home</li><li><a href="http://localhost/tp/foodbasket/account/login">Login</a> </li><li class="active">Home</li></ol>
            </nav>
        </section>

In other words, there are repeated active breadcrumbs... What do you think about this?

nobuti commented 9 years ago

Again, I don't see the logic of having repeating items in the breadcrumb, that is not the case use scenario.

tjapro commented 9 years ago

I see that you aren't understand how I'm doing the program. So, I show you how I do it:

    /**
     * Index Page for this controller.
     */
    public function index() {
        $data = array('title' => $this -> title);
        $role = ($this -> session -> userdata('role') === FALSE) ? '' : ucwords($this -> session -> userdata('role'));
        $user['name'] = $this -> session -> userdata('name');
        $user['email'] = $this -> session -> userdata('email');
        if ($this -> alert['alert_type'] == '') {
            $this -> alert['alert_type'] = $this -> alert_hidden;
        }

        $this -> breadcrumbs -> unshift('Home', '/');
        $data['breadcrumbs'] = $this -> breadcrumbs -> show();

        $this -> load -> view('templates/header', $data);
        $this -> load -> view('templates/menu' . $role, $user);
        $this -> load -> view('account/' . $this -> page, array_merge($this -> errors, $this -> alert));
        $this -> load -> view('templates/footer', $data);
    }

public function login(){
...
$this -> breadcrumbs -> push('Login', '/account/login');
$this -> index();
...
}

PS: The the login function is the first to be called.

nobuti commented 9 years ago

Ok, I now understand, but IMO this in an edge case, you can solve the issue refactoring the call methods.

tjapro commented 9 years ago

Please, can me give an example to refactoring the call methods, or is better change the code as above showed?

nobuti commented 9 years ago

Ok, I see the bug now, apologies. But the code above still seems a bit strange. I'd try to isolate the breadcrumb construction inside every method. But I don't know the context of the project.