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

docs: Use definition lists for parameter descriptions to enable full markdown support be #901

Merged
merged 10 commits into from
Dec 18, 2023
Merged

docs: Use definition lists for parameter descriptions to enable full markdown support be #901

merged 10 commits into from
Dec 18, 2023

Conversation

gadenbuie
Copy link
Collaborator

@gadenbuie gadenbuie commented Dec 12, 2023 •
edited
Loading

Fixes #900

This PR brings full markdown support for parameter descriptions in the py-shiny docs. We originally tried using a grid table for writing the parameters table (and just for the parameters table), but I think definition lists are much easier to read.

This PR also adds some CSS to the dev docs to nudge the spacing of the definition list. We'll need to make sure similar rules are applied to the primary docs when we do our next release.

Updated documentation

  • ui.layout_columns()
  • ui.layout_column_wrap()
  • Other functions...?

cc: check this out, @machow!

@gadenbuie gadenbuie marked this pull request as ready for review December 12, 2023 21:39
@machow
Copy link
Collaborator

machow commented Dec 12, 2023

I'm super curious about the lingering character quarto is adding?! I'm going to work on bringing in support for list-style parameter tables, similar to https://pandas.pydata.org/docs/reference/api/pandas.DataFrame.html#pandas.DataFrame

WDYT of that style for parameters / table-y things? (obv for down the road, this PR is beautiful)

@gadenbuie
Copy link
Collaborator Author

I'm going to work on bringing in support for list-style parameter tables, similar to pandas.pydata.org/docs/reference/api/pandas.DataFrame.html#pandas.DataFrame

WDYT of that style for parameters / table-y things? (obv for down the road, this PR is beautiful)

Oh! I love this idea. Personally, I prefer this approach over using tables for parameters. On our website it might look something like this.

Before

image

After

image

@machow
Copy link
Collaborator

machow commented Dec 13, 2023

Oh wow, that was fast! @has2k1 has definition lists implemented in plotnine for his quarto migration too: https://has2k1.github.io/plotnine/reference/geom_bar.html#plotnine.geoms.geom_bar

I'll try and add an option for these in the next few days in quartodoc, based on all y'all's work!

gadenbuie
gadenbuie commented Dec 13, 2023
View reviewed changes
docs/_renderer.py Outdated Show resolved Hide resolved
gadenbuie
gadenbuie commented Dec 13, 2023
View reviewed changes
docs/_renderer.py Outdated Show resolved Hide resolved
has2k1
has2k1 reviewed Dec 14, 2023
View reviewed changes
docs/_renderer.py Outdated Show resolved Hide resolved
@gadenbuie gadenbuie changed the title docs: Use grid tables for parameter descriptions to enable full markdown support docs: Use definition lists for parameter descriptions to enable full markdown support be Dec 14, 2023
@gadenbuie
Copy link
Collaborator Author

gadenbuie commented Dec 15, 2023 •
edited
Loading

@schloerke thanks to Michael's suggestion, I tried definition lists and think they're a big improvement. The custom writer here works and this PR is ready to merge after your review.

I did try upgrading to quartodoc 0.7.2 and griffe 0.33 to see if we could use DefinitionList from quartodoc.pandoc. It would be very nice and it'd be as simple as replacing our custom dl renderer with all of str(DefinitionList(rows)).

That said, I ran into issues trying to render the docs with the newer package versions. If you see an easy fix that'd be great, but we could just as easily merge this now and come back to a quartodoc upgrade later.

Thanks to @has2k1, the fix to upgrade to griffe 0.33 was easy!

@gadenbuie gadenbuie removed the request for review from cpsievert December 18, 2023 14:47
schloerke
schloerke approved these changes Dec 18, 2023
View reviewed changes
Copy link
Collaborator

@schloerke schloerke left a comment

Choose a reason for hiding this comment

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

LGTM

In a followup PR, please look at how the styling for the code in the definition term has changed from before to the new look. It also looks like all of the fonts are of a heavier weight

@gadenbuie
Copy link
Collaborator Author

In a followup PR, please look at how the styling for the code in the definition term has changed from before to the new look. It also looks like all of the fonts are of a heavier weight

That's the difference between the dev docs and the released/published docs

@gadenbuie gadenbuie merged commit 3bc7117 into main Dec 18, 2023
26 checks passed
@gadenbuie gadenbuie deleted the docs/parameters-grid-table branch December 18, 2023 17:01
schloerke added a commit that referenced this pull request Dec 18, 2023
@schloerke
Merge branch 'main' into garrettgman/main
6a48c66 
* main: (24 commits)
  docs: Use definition lists for parameter descriptions to enable full markdown support (#901)
  docs(layout_columns): Add example app (#903)
  fix(layout_columns): Remove use of enum for breakpoints (#912)
  Express: don't automatically put strings in a `<pre>` block (#905)
  Deprecate `nav()` in favor of `nav_panel()` (#876)
  chore: Remove unused import
  refactor(layout_column_wrap): Use helper for wrapping children (#910)
  fix(layout_columns): Don't apply fillable class to layout container (#909)
  Add output to React template (#908)
  fix(layout_columns): Add missing `bslib-mb-spacing` class (#906)
  Custom component template updates (#874)
  fix(layout_columns): If the breakpoint name is an enum, need to collect the value (#902)
  chore: exclude `_dev/` folder (#897)
  feat: Add `ui.layout_columns()` (#856)
  Update VS Code settings file
  Express: change default page function to page_fixed (#892)
  Better messages for `AttributeError` in Shiny Express (#888)
  Fix 0.6.0 release date
  Fix failing express tests (#887)
  Explain why function is called twice
  ...
schloerke added a commit that referenced this pull request Dec 19, 2023
@schloerke
Merge branch 'main' into drop_fn_session_param
65829a3 
* main: (76 commits)
  Add shiny/templates to MANIFEST.in files (#926)
  docs: Function Reference Proofreads (#919)
  Fix failing deploy tests (#925)
  Add GitHub sources to shiny create
  Make changelog styling more consistent
  Function reference edits (#857)
  docs: Use definition lists for parameter descriptions to enable full markdown support (#901)
  docs(layout_columns): Add example app (#903)
  fix(layout_columns): Remove use of enum for breakpoints (#912)
  Express: don't automatically put strings in a `<pre>` block (#905)
  Deprecate `nav()` in favor of `nav_panel()` (#876)
  chore: Remove unused import
  refactor(layout_column_wrap): Use helper for wrapping children (#910)
  fix(layout_columns): Don't apply fillable class to layout container (#909)
  Add output to React template (#908)
  fix(layout_columns): Add missing `bslib-mb-spacing` class (#906)
  Custom component template updates (#874)
  fix(layout_columns): If the breakpoint name is an enum, need to collect the value (#902)
  chore: exclude `_dev/` folder (#897)
  feat: Add `ui.layout_columns()` (#856)
  ...
schloerke added a commit that referenced this pull request Dec 19, 2023
@schloerke
Merge branch 'main' into fn_signatures_for_docs
aecc5d8 
* main: (36 commits)
  Add shiny/templates to MANIFEST.in files (#926)
  docs: Function Reference Proofreads (#919)
  Fix failing deploy tests (#925)
  Add GitHub sources to shiny create
  Make changelog styling more consistent
  Function reference edits (#857)
  docs: Use definition lists for parameter descriptions to enable full markdown support (#901)
  docs(layout_columns): Add example app (#903)
  fix(layout_columns): Remove use of enum for breakpoints (#912)
  Express: don't automatically put strings in a `<pre>` block (#905)
  Deprecate `nav()` in favor of `nav_panel()` (#876)
  chore: Remove unused import
  refactor(layout_column_wrap): Use helper for wrapping children (#910)
  fix(layout_columns): Don't apply fillable class to layout container (#909)
  Add output to React template (#908)
  fix(layout_columns): Add missing `bslib-mb-spacing` class (#906)
  Custom component template updates (#874)
  fix(layout_columns): If the breakpoint name is an enum, need to collect the value (#902)
  chore: exclude `_dev/` folder (#897)
  feat: Add `ui.layout_columns()` (#856)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@has2k1 has2k1 has2k1 left review comments

@schloerke schloerke schloerke approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use grid table for parameters in quartodoc
4 participants
@gadenbuie @machow @schloerke @has2k1