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.

Sign up for GitHub

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

Jump to bottom

Transfer search parameters from page URL to jupyterlite #108

Merged
merged 7 commits into from
Sep 11, 2023
Merged

Transfer search parameters from page URL to jupyterlite #108

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d65dbbb
Add directive to transfert URL search parameters from the documentati…
brichet Sep 7, 2023
137a1ea
Add documentation on the directive
brichet Sep 7, 2023
31cd4ce
prettier
brichet Sep 7, 2023
f7a590e
Add an option to transfer all the parameters
brichet Sep 8, 2023
1169787
Allow True, False or ["param1", "param2"] as options
brichet Sep 8, 2023
8504851
Adds regex to checks parameters
brichet Sep 8, 2023
a8e8639
Update jupyterlite_sphinx/jupyterlite_sphinx.py
brichet Sep 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/directives/jupyterlite.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,8 @@ You can also pass a Notebook file to open automatically:
:prompt: Try JupyterLite!
:prompt_color: #00aa42
```


The directive `search_params` allows to transfer some search parameters from the documentation URL to the Jupyterlite URL.\
Jupyterlite is than able to these parameters from its own URL.\
For example `:search_params: param1, param2` will transfer the parameters *param1* and *param2*.
16 changes: 16 additions & 0 deletions jupyterlite_sphinx/jupyterlite_sphinx.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,19 @@ window.jupyterliteShowIframe = (tryItButtonId, iframeSrc) => {
tryItButton.classList.remove('jupyterlite_sphinx_try_it_button_unclicked');
tryItButton.classList.add('jupyterlite_sphinx_try_it_button_clicked');
}

window.jupyterliteConcatSearchParams = (iframeSrc, params) => {
const baseURL = window.location.origin;
const iframeUrl = new URL(iframeSrc, baseURL);

let pageParams = new URLSearchParams(window.location.search);

params.forEach(param => {
value = pageParams.get(param);
if (value !== null) {
iframeUrl.searchParams.append(param, value);
}
});

return iframeUrl.toString().replace(baseURL, '');
}
34 changes: 27 additions & 7 deletions jupyterlite_sphinx/jupyterlite_sphinx.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ def __init__(
height="100%",
prompt=False,
prompt_color=None,
search_params=[],
**attributes,
):
super().__init__(
Expand All @@ -60,10 +61,12 @@ def __init__(
height=height,
prompt=prompt,
prompt_color=prompt_color,
search_params=search_params,
)

def html(self):
iframe_src = self["iframe_src"]
search_params = self["search_params"]

if self["prompt"]:
prompt = (
Expand All @@ -76,13 +79,24 @@ def html(self):
placeholder_id = uuid4()
container_style = f'width: {self["width"]}; height: {self["height"]};'

return (
f"<div class=\"jupyterlite_sphinx_iframe_container\" style=\"{container_style}\" onclick=window.jupyterliteShowIframe('{placeholder_id}','{iframe_src}')>"
f' <div id={placeholder_id} class="jupyterlite_sphinx_try_it_button jupyterlite_sphinx_try_it_button_unclicked" style="background-color: {prompt_color};">'
f" {prompt}"
" </div>"
"</div>"
)
return f"""
<div
class=\"jupyterlite_sphinx_iframe_container\"
style=\"{container_style}\"
onclick=\"window.jupyterliteShowIframe(
'{placeholder_id}',
window.jupyterliteConcatSearchParams('{iframe_src}',{search_params})
)\"
>
<div
id={placeholder_id}
class=\"jupyterlite_sphinx_try_it_button jupyterlite_sphinx_try_it_button_unclicked\"
style=\"background-color: {prompt_color};\"
>
{prompt}
</div>
</div>
"""

return (
f'<iframe src="{iframe_src}"'
Expand Down Expand Up @@ -233,6 +247,7 @@ class _LiteDirective(SphinxDirective):
"theme": directives.unchanged,
"prompt": directives.unchanged,
"prompt_color": directives.unchanged,
"search_params": directives.unchanged,
}

def run(self):
Expand All @@ -242,6 +257,10 @@ def run(self):
prompt = self.options.pop("prompt", False)
prompt_color = self.options.pop("prompt_color", None)

search_params = list(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the search_param directive here for whitelisting the parameters that we want to transfer? Would there be an issue transferring all of them blindly?

Copy link
Contributor Author

@brichet brichet Sep 7, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure to understand.
It's here to build a python list from the string, but yes we could probably transfer the whole string and directly build a javascript array from it.
Do you think of an other way ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I mean is the following (removing the search_param directive and transferring all query parameteres to the iframe)

window.jupyterliteConcatSearchParams = (iframeSrc) => {
  const baseURL = window.location.origin;
  const iframeUrl = new URL(iframeSrc, baseURL);

  let pageParams = new URLSearchParams(window.location.search);

  pageParams.keys().forEach(param => {
    value = pageParams.get(param);
    if (value !== null) {
      iframeUrl.searchParams.append(param, value);
    }
  });

  return iframeUrl.toString().replace(baseURL, '');
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure it makes sense to forward all query parameters. But it would simplify the code and user API.

Copy link
Contributor Author

@brichet brichet Sep 7, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK thanks, I understand your first comment. Yes, the idea is to whitelist the parameters.

I am not sure it makes sense to forward all query parameters. But it would simplify the code and user API.

It's better to have at least the option (like a flag) to let user allow the transfer or not.
I don't know which is the best.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could add an option to transfer all the parameters(=all for example, since = can probably not be used as parameter name), or use only a boolean value to transfer all or none.

Do you have an opinion on it @martinRenou ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, we could have the search_params be either a list of strings (whitelist) or a boolean (True=transfer all, False=transfer none).

Also, I wonder if this could be implemented as an option for other directives, instead of a global search_params directive, like:

.. jupyterlite::
   :search_params: True

Or

.. replite::
   :search_params: ["theme", "code"]

This way it will be more granular

map(str.strip, self.options.pop("search_params", "").split(","))
)

source_location = os.path.dirname(self.get_source_info()[0])

prefix = os.path.relpath(
Expand Down Expand Up @@ -276,6 +295,7 @@ def run(self):
height=height,
prompt=prompt,
prompt_color=prompt_color,
search_params=search_params,
lite_options=self.options,
)
]
Expand Down