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

Add custom link function #91

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

minibikini
Copy link

@minibikini minibikini commented Oct 4, 2019

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
Copy link

feliperenan commented Dec 4, 2019

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
Copy link
Owner

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
Copy link
Contributor

mcrumm commented Dec 17, 2019

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

@speeddragon
Copy link

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

@arfl
Copy link

arfl commented Mar 26, 2020

Any news on this?

@jrissler
Copy link

Tested this today - worked great here 😃

@ssbb
Copy link

ssbb commented Jun 16, 2020

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

@polmiro
Copy link

polmiro commented Oct 6, 2020

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

@jrissler
Copy link

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

fadzril added a commit to Robugroup/scrivener_html that referenced this pull request Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants