[Freeipa-devel] [PATCH] 647-651 [webui] Make utility section of navigation extensible

[Petr Vobornik]

On 23.6.2014 18:35, Endi Sukma Dewata wrote:
> On 6/23/2014 8:15 AM, Petr Vobornik wrote:
>>> 1. I'm not sure if we really need a HashCreator. Ideally the router
>>> should map a hash to a page. Links to another page can be hardcoded too
>>> (and substitute the parameters).
>>
>> The main purpose of a hash creator is to update hash when a facet state
>> changes. This change can be initiated from the facet itself, e.g. when
>> searching in a search facet. Removing the hash creator would make facets
>> dependent on navigation specifics.
>
> I was thinking if the facet itself is changing the state, it will only
> change the query part of the URL. The base facet URL itself is assigned
> by the router. I think we used to put all UI states in the URL so that's
> why we needed a hash creator.

My thinking was that the facet shouldn't be even aware of the hash or 
routing.

>
>> I would agree if it was related only to links. But even then it would be
>> better to have a method which would create the hash (for the one which
>> shares the same pattern) to reduce code reuse. In any case, it's
>> possible to hardcode hash parts in links if one needs to.
>
> It's more about removing dependencies between pages. For example, to
> link from one page to another we use something like this:
>
>    navigation.show_entity(entity.name, facetname, [pkey]);
>
> This means we're explicitly handling the link. And this also puts
> additional restrictions such as the target page must be an entity page
> which has one level sub page called 'facetname' that takes one primary
> key. If the target page is not an entity page we'd have to use a
> different method.
>
> I think it would be cleaner if the link can be defined in a (future)
> template like this:
>
>    <a href="#/{entity}/{facet}/{pkey}">
>

How is this different? You still have to pass the entity, facet name and 
pkey. You just moved the logic form hash creator to the template. And I 
can bet that doing something more difficult there will be a pain.

Also it doesn't work for redirection initiated by other means then a 
link, e.g., when something gets deleted.

If one wants to create a link in a template he could use some link 
helper which would internally call the hash creator.

> Note that the template is used to generate the links only. When the user
> clicks the link we rely on the browser, or other routing libraries, to
> handle it so less code to maintain and it puts the UI workflow in the
> hands of UI designers.

Keeping facet and widget internals out of direct hash modification 
allows to embed the whole app or just some parts of it into other app. 
Or we can have two different or same facets displayed next to each 
other. Atm this is not an argument since we do not make it with this use 
case in mind. The same applies for code modifications by designers.

>
>>> 2. Ideally there should be no entity-specific navigation code. All
>>> routing should be handled in a generic way. This probably depends on the
>>> entity-facet separation that we discussed previously. So routes like
>>> this:
>>>
>>>    /e/:entity/:facet/:pkeys/*args
>>>
>>> should be replaced by individual routes for each entity:
>>>
>>>    /user/:facet/:pkeys/*args
>>>    /group/:facet/:pkeys/*args
>>
>> Yes, but IMHO the hash prefix is a detail. It would be more important if
>> it would be a REST API where it's a core aspect.
>
> Right, a REST URL probably would look like:
>
>    /ipa/ui/users/{facet}/{pkey}?{query}
>
> And ideally if you open a REST URL in a browser you should get a UI. So
> anything that brings the UI closer to this would be great.
>
>>> 4. The args/query in the navigation paths should be separated by a more
>>> standard delimiter "?" instead of "//". For example:
>>>
>>>    /ipa/ui/#/e/user/search//filter=test
>>>
>>> should be replaced with:
>>>
>>>    /ipa/ui/#/e/user/search?filter=test
>>
>> note that // is a result of /:pkeys/, where :pkeys is '' Therefore a
>> simple replacement would lead to
>>      /ipa/ui/#/e/user/search/?filter=test
>>
>> Please file a ticket, if you think it's important.
>
> I was originally thinking this to be a way to mimic the REST URL, but
> now I'm not sure if it would interfere with URL parsing because in this
> case the ? will be part of the URL hash, not URL query, so maybe we
> should just leave it for now.
>
>> Lets say that we would implement #2 and #4. Would we also want to keep
>> the old routes for backwards compatibility? (main purpose of hashes is
>> to make the page bookmarkable). IMHO people don't care about #2 and #4
>> much, but they care about broken bookmarks.
>
> The hashes are probably more useful to make the browser's Back button
> functional. Bookmarking is probably a secondary benefit, but are there
> pages in the UI that people would likely to bookmark? An admin probably
> would just bookmark the main URL, not a particular page in the UI.
> Backward compatibility of routing is nice, but I don't think it's a hard
> requirement.
>


-- 
Petr Vobornik