Skip to content

Use dataclasses instead of extra_classes#597

Merged
damianavila merged 7 commits intopydata:masterfrom
choldgraf:enh-version-autolabel
Mar 7, 2022
Merged

Use dataclasses instead of extra_classes#597
damianavila merged 7 commits intopydata:masterfrom
choldgraf:enh-version-autolabel

Conversation

@choldgraf
Copy link
Copy Markdown
Collaborator

@choldgraf choldgraf commented Feb 24, 2022

This changes our version switcher to use dataclass entries for all dropdown links and the button itself, rather than having users manually add classes to the JSON entries.

closes #596

tupui
tupui approved these changes Feb 24, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@tupui tupui left a comment

Choose a reason for hiding this comment

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

Wooo thanks! This looks great and I think I get the doc to use it properly! Approving for what it's worth 😃

damianavila
damianavila reviewed Feb 25, 2022
View reviewed changes
Comment thread docs/_static/custom.css
damianavila
damianavila reviewed Feb 25, 2022
View reviewed changes
Comment thread docs/conf.py Outdated
damianavila
damianavila reviewed Feb 25, 2022
View reviewed changes
Comment thread docs/user_guide/configuring.rst Outdated
damianavila
damianavila reviewed Feb 25, 2022
View reviewed changes
Comment thread docs/user_guide/configuring.rst Outdated
@tupui tupui mentioned this pull request Feb 25, 2022
Merged
@choldgraf choldgraf force-pushed the enh-version-autolabel branch from 434cd25 to d597406 Compare February 25, 2022 20:49
@choldgraf
Copy link
Copy Markdown
Collaborator Author

choldgraf commented Feb 25, 2022

OK - addressed each of @damianavila's suggestions, and also rebased on master so that we can compare the behavior.

@damianavila
Copy link
Copy Markdown
Collaborator

damianavila commented Feb 26, 2022 •
edited
Loading

Master is currently broken given #574 (comment)

I have a fix in place in an upcoming PR: #599.

Let's merge that one first and rebase this one so we can effectively compare.

@choldgraf choldgraf mentioned this pull request Feb 26, 2022
Merged
@choldgraf
Copy link
Copy Markdown
Collaborator Author

choldgraf commented Mar 5, 2022

I've merged #599 and updated this branch as well, so that we can compare the changes in code + functionality more directly. Would love feedback on whether this PR is the right direction to go, and anything we'd need to change here.

damianavila
damianavila reviewed Mar 5, 2022
View reviewed changes
Comment thread docs/user_guide/configuring.rst
@choldgraf
Copy link
Copy Markdown
Collaborator Author

choldgraf commented Mar 6, 2022 •
edited
Loading

Updated the example @damianavila - also updated the CSS per your suggestions

damianavila
damianavila approved these changes Mar 7, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@damianavila damianavila left a comment

Choose a reason for hiding this comment

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

LGTM now!

@damianavila damianavila merged commit 9481c3d into pydata:master Mar 7, 2022
@damianavila
Copy link
Copy Markdown
Collaborator

damianavila commented Mar 7, 2022

And I merged it 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@damianavila damianavila damianavila approved these changes

+1 more reviewer

@tupui tupui tupui approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Use a data- attribute instead of CSS classes for the active version in version switcher

3 participants

@choldgraf @damianavila @tupui