Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Batching for content listing tile #56

Open
Martronic-SA opened this issue Dec 15, 2016 · 9 comments
Open

Batching for content listing tile #56

Martronic-SA opened this issue Dec 15, 2016 · 9 comments

Comments

@Martronic-SA
Copy link

It seems that content listing doesn't provide batching by default.
It should! doesn'it?
There's missing "Item count" like in collections.

@thomasmassmann
Copy link
Member

If we enable batching for content listing tiles, some questions need to be answered before we can start implementing it:

  • Are multiple content listing tiles allowed on a page (I think yes), and if,
  • how can we generate URLs users can bookmark, if a content item has multiple content listing tiles?
  • Should we take the id of the tile into account, e.g. ?b_start_5a3076bb67054a04b:int=10?
  • Or are we going with a JS only version using ajax_load and data-pat-contentloader?
  • Or can we have the AJAX version and still bookmarkable URLs?

@datakurre, what's your opinion on that?

@datakurre
Copy link
Member

datakurre commented Dec 19, 2017

@tmassman I would vote for JS-only version, where batching within tiles has direct urls for the tile view. It should work with pat-contentloader, even without ajax-load. It might even be good for accessibility without JS, because without JS it would open just the listing tile without any confusing surroundings. It would also be fast, because loading only the tile view with AJAX should be fast.

My vote for that because tiles should ideally be independent (not relying on being able to access “paren request” somehow).

Bookmarking is harder. It would the require pat-contentloader to append something to browser url that it could use later to reload the correct content on load. So that is actually a more generic bookmarkability issue with the patterns

Alternative “pure” version could use those prefixed query parameters on main page, but require a new transform that would append batching parameters to tile URLs before tile merge. It would work without AJAX and be bookmarkable, but would also be much slower.

@thomasmassmann
Copy link
Member

@datakurre, sounds good. Thanks for your input!

@djay
Copy link
Member

djay commented Feb 28, 2018 via email

@datakurre
Copy link
Member

That latest option is currently the only easy one.

Only clean way to pass information from the render context to tile is querystring and there’s not much space in it (and danger to break tile if cannot handle unexpected parameters).

@djay
Copy link
Member

djay commented Mar 1, 2018

@datakurre I see the potential problem but it doesn't seem right to prevent a better UX because of such an internal detail.
The reason I case is that there are other use cases for tiles accepting request data like the custom search forms using tiles. Also the progressive enhancement thing.

  • Are you sure the url length is really an issue? Isn't it going via subrequest in most cases? And even if you use SSI then I'm pretty sure most proxies don't enforce the url length, thats more a browser thing.
  • The other consideration is that the tile author has control over the param length so could warn or encode params to reduce the length to ensure that the additional param for paging could be handled.
  • Another option is to use a post request for next and previous. Thats perhaps not a great option for SEO though.
  • finally there is the option to make listing tiles persistent. If there is a risk of the params being too long due to adding a &p=10 then you have that risk regardless.

@datakurre
Copy link
Member

I agree that URL length might be just a theoretical issue.

So, we need is to define a prefixing syntax for passing GET query-parameters from the main requests to the tile request. The easiest way would be to limit this support for tiles with ids (which covers all tiles created through Mosaic Editor).

Semantically I see it as the same as prefixing HTML forms to allow multiple forms on the same without conflicting fields or actions.

So, should

?b_start:int=20

become

?tile-018e75ef01044508be35d4bae8ed1caf-b_start:int=20

or something better?

Tile would still not know on which URL it is being rendered (this is a more generic issue with tiles). Of course, ff tile would just render relative URLs starting with ?tile-018e75ef01044508be35d4bae8ed1caf-b_start:int=20 it would on default views that is the most common use case.

Possibly, if the current view is not a default view, tile transform should also pass the name of the current view to tile in some documented query parameter (I recall that viewlet tiles already support viewname parameter for rendering viewlets registered for non default views).

@djay
Copy link
Member

djay commented Mar 2, 2018

@datakurre or another idea could be that the tile specifies what params it should be passed and there is some matching that goes on. e.g. the definition of the listing tile includes b_start:int=0 and if that exists in the top level request then its gets replaced. There could be support for a **args equivalent to get everything. A bit like python functions. Not sure if this makes ESI much more complicated though.

@djay
Copy link
Member

djay commented Mar 2, 2018

on 2nd thoughts that is a bit of security issue as it allows anyone to change a tiles params. Maybe they need to be at the end, or have a special value? e.g. ...&b_start:int=_REQUEST_. or ...&b_start:int:request=0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants