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

Implement/improve nav() API by implementing in Python instead of React/JSX #136

Merged
merged 15 commits into from
Apr 28, 2022

Conversation

cpsievert
Copy link
Collaborator

@cpsievert cpsievert commented Apr 25, 2022

After investigating what the nav() API implementation would look like in JSX and web components, it seems better to just implement it in Python, for a few different reasons:

  1. There are at least a couple known cases where putting HTML inside JSX leads to different/undefined behavior, which is really unfortunate if the end user is not aware the function they're using wraps a JSX component.

    • At the moment, we don't have as many strong reasons to avoid web components (other than the elements wanting to be defined after initial page load).
  2. Compared to what we have in R, a significant amount of complexity can be avoided by: assuming Bootstrap 5+, using an object oriented approach, and having proper type hinting (will also help help users know about mistakes in their code without making a bunch of run-time checks)

TODO:

@cpsievert cpsievert requested a review from jcheng5 April 25, 2022 19:32
@cpsievert cpsievert force-pushed the server-side-navs branch 2 times, most recently from 5310a0b to ca38ac1 Compare April 25, 2022 20:00
@cpsievert cpsievert force-pushed the server-side-navs branch 2 times, most recently from ecbe0af to bfdc2e9 Compare April 26, 2022 19:36
@cpsievert cpsievert marked this pull request as draft April 26, 2022 20:55
@cpsievert cpsievert mentioned this pull request Apr 26, 2022
4 tasks
…ith a tagify() method (so we can add other useful methods in the future); add a tagify() method to Nav/NavMenu as well that raises informative error
@cpsievert cpsievert changed the title Switch from React/JSX to Python for constructing nav() markup Implement/improve nav() API by implementing in Python instead of React/JSX Apr 27, 2022
@cpsievert cpsievert marked this pull request as ready for review April 27, 2022 21:42
@cpsievert cpsievert merged commit d2e3fb9 into main Apr 28, 2022
@cpsievert cpsievert deleted the server-side-navs branch April 29, 2022 14:41
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.

1 participant