mgwidmann / scrivener_html

HTML view helpers for Scrivener
MIT License
125 stars 208 forks source link

Add custom link function #91

Open minibikini opened 5 years ago

minibikini commented 5 years ago

Adds a new :link_fun option to set the link function. Defaults to Phoenix.HTML.link/2.

This will allow to use Scrivener with Phoenix LiveView which has own link generation function for "live navigation".

I've replaced some common arguments with page_opts map and now not sure it was needed :) If you don't like it I'll try to turn it back.

P.S. I've been using Scrivener for years! Such a handy library, thank you!

feliperenan commented 5 years ago

Hi 👋

We are also using Scrivener here & Live View. Comparing with another solution I'd rather this one since it's more flexible - as far as I understood we could even define live_link as default. But both are good IMO and it's up to the maintainers to decide 😅.

What I may add here is that I tested both and they worked well in our application. Just want to know if there are plans to move forward with Live View support and if so, what else we can do to help with that 😄.

Thanks 🙌

mgwidmann commented 4 years ago

This looks fine to me. I don't have an application to try it out in other than the unit tests, can anyone confirm this branch is backwards compatible against a real application?

mcrumm commented 4 years ago

@mgwidmann :+1: confirmed backwards compatible, and the link override looks good. I look forward to seeing this land!

speeddragon commented 4 years ago

I think this should be merged and created a new release.

arfl commented 4 years ago

Any news on this?

jrissler commented 4 years ago

Tested this today - worked great here 😃

ssbb commented 4 years ago

@mgwidmann We using this branch for both normal pagination and LV-based. Works great. It would be cool if you can merge this.

polmiro commented 4 years ago

Tested with my application and worked perfectly, still pointing to the forked version though. Looking forward to see it merged 🍻

jrissler commented 3 years ago

@mgwidmann what would it take to get some of these open PR's merged in?