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

Link preview: allow customization of link query selector #554

Open
agjohnson opened this issue Feb 22, 2025 — with Front Integration · 3 comments
Open

Link preview: allow customization of link query selector #554

agjohnson opened this issue Feb 22, 2025 — with Front Integration · 3 comments
Labels
Feature New feature Needed: design decision A core team decision is required

Comments

Copy link
Contributor

agjohnson commented Feb 22, 2025

Allow for user customization of the query selector used for selecting link elements in the DOM. We already allow for link selector customization based on the underlying doc tool, so we're not far off. This would require a configuration option in the dashboard, and the default value could still be our automatic guess.

Front logo Front conversations

@agjohnson agjohnson added Feature New feature Needed: design decision A core team decision is required labels Feb 22, 2025 — with Front Integration
@humitos
Copy link
Member

humitos commented Feb 24, 2025

We already discuss this in the past. My initial implementation supported this use case but we decided to not move forward with that and use auto-detection instead.

There is a lot of discussion in #433 and readthedocs/readthedocs.org#11767 that we should review to understand our previous decisions and decided whether or not it makes sense now.

@agjohnson
Copy link
Contributor Author

I'm not describing the root selector described in those PRs, I'm describing the link selector, ie:

const selector = docTool.getLinkSelector();

The link selector getLinkSelector seems to be really close to what I described above, it just doesn't read a setting from addons configuration. The getLinkSelector seems like it would need to look like the getRootSelector logic above it:

addons/src/linkpreviews.js

Lines 247 to 250 in 4600c99

const rootSelector =
this.config.addons.options.root_selector || docTool.getRootSelector();
const selector = docTool.getLinkSelector();

Is there a specific issue you are trying to describe with this approach?

If we really needed to, getLinkSelector could have implicit knowledge of the root selector (provided by configuration or by our default rules) so that users only need to provide a value like a.internal[href=...] instead of div[role=main] a.internal[href=...] as the selector.

All together:

const rootSelector =
  this.config.addons.options.root_selector || docTool.getRootSelector();

let selector = docTool.getLinkSelector();
if (this.config.addons.linkpreviews.selector) {
  selector = `${rootSelector} ${this.config.addons.linkpreviews.selector}`
}

@humitos
Copy link
Member

humitos commented Feb 25, 2025

The link selector getLinkSelector seems to be really close to what I described above, it just doesn't read a setting from addons configuration. The getLinkSelector seems like it would need to look like the getRootSelector logic above

Yes.

so that users only need to provide a value like a.internal[href=...] instead of div[role=main] a.internal[href=...] as the selector

I would avoid this. If the user wants to change the CSS selector for the links preview, I would leave them to do it freely. This is an advance usage and I wouldn't like to be in middle. I'm sure there are going to be user that don't want use our heuristic because different reasons and they will be blocked by us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature Needed: design decision A core team decision is required
Projects
Status: Planned
Development

No branches or pull requests

2 participants