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

UPDATE: pydata sphinx theme v0.7.1 #406

Merged
merged 7 commits into from
Oct 16, 2021

Conversation

ocefpaf
Copy link
Contributor

@ocefpaf ocefpaf commented Oct 1, 2021

Hopefully I won't break anything with the test change.

@welcome
Copy link

welcome bot commented Oct 1, 2021

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.

Welcome to the EBP community! 🎉

@choldgraf
Copy link
Member

Nice! thanks for this :-) one quick thought, I think we can now remove our custom previous/next templating and CSS, since we've now upstreamed that to the pydata theme:

{# Define macro for re-use #}
{% macro prev_next(prev, next, prev_title='', next_title='') %}
{%- if prev %}
<div id="prev">
<a class="left-prev" href="{{ prev.link|e }}" title="{{ _('previous') }} {{ _('page') }}">
<i class="prevnext-label fas fa-angle-left"></i>
<div class="prevnext-info">
<p class="prevnext-label">{{ _("previous") }}</p>
<p class="prevnext-title">{{ prev_title or prev.title }}</p>
</div>
</a>
</div>
{%- endif %}
{%- if next %}
<div id="next">
<a class="right-next" href="{{ next.link|e }}" title="{{ _('next') }} {{ _('page') }}">
<div class="prevnext-info">
<p class="prevnext-label">{{ _("next") }}</p>
<p class="prevnext-title">{{ next_title or next.title }}</p>
</div>
<i class="prevnext-label fas fa-angle-right"></i>
</a>
</div>
{%- endif %}
{% endmacro %}
{# Add previous / next buttons to page #}
<div class='prev-next-bottom'>
{{ prev_next(prev, next) }}
</div>

any interest in trying that out as part of this?

@ocefpaf
Copy link
Contributor Author

ocefpaf commented Oct 1, 2021

any interest in trying that out as part of this?

Deleting code? YES! That is my favorite black hole to fall into. I'll report back if I crash and burn in the process.

@chrisjsewell
Copy link
Member

think we can now remove our custom previous/next templating and CSS

To check, does it do translations?

@ocefpaf
Copy link
Contributor Author

ocefpaf commented Oct 1, 2021

BTW, can I get a hacktoberfest-accepted label ;-p

@choldgraf
Copy link
Member

choldgraf commented Oct 1, 2021

@chrisjsewell good catch, it looks like the original PR got modified before it was merged and the code needs tweaking. Will make a PR:

pydata/pydata-sphinx-theme#486

@ocefpaf label applied :-) though note we should not merge this PR until a new PyData theme is released, if we wish to make the templating changes in this PR

@ocefpaf
Copy link
Contributor Author

ocefpaf commented Oct 1, 2021

if we wish to make the templating changes in this PR

That is up to you. Should I revert the last commit or wait?

@choldgraf
Copy link
Member

I think it's fine so long as we don't merge the PR. We can cut a new release on the pydata theme quickly so i think we can just get this one ready to go

@choldgraf
Copy link
Member

I've just released a new version of the pydata theme (0.7.1) and bumped the patch version here. Let's see how it builds

@choldgraf
Copy link
Member

choldgraf commented Oct 2, 2021

The prev/next buttons look great! But, I have also noticed some other regressions. For example, see the margin content block:

image

I don't think we're inheriting that from the pydata theme directly, but seem to be getting it from sphinx basic.css file. Here's the CSS:

div.sidebar, aside.sidebar {
    margin: 0 0 0.5em 1em;
    border: 1px solid #ddb;
    padding: 7px;
    background-color: #ffe;
    width: 40%;
    float: right;
    clear: right;
    overflow-x: auto;
}

@ocefpaf wanna try just adding some CSS to remove that yellow background? And then we should do a quick look over the kitchen sink docs to make sure nothing else has changed unexpectedly

@ocefpaf
Copy link
Contributor Author

ocefpaf commented Oct 5, 2021

@ocefpaf wanna try just adding some CSS to remove that yellow background? And then we should do a quick look over the kitchen sink docs to make sure nothing else has changed unexpectedly

I'm definitely out of my environment for that change. I'll probably just break it more. Sorry but CSS is a mystery to me.

@choldgraf
Copy link
Member

Sounds good @ocefpaf - if you don't mind I'll push to this branch and we can merge it together

@ocefpaf
Copy link
Contributor Author

ocefpaf commented Oct 5, 2021

Sounds good @ocefpaf - if you don't mind I'll push to this branch and we can merge it together

Not at all. Push all the commits you need.

@ocefpaf
Copy link
Contributor Author

ocefpaf commented Oct 14, 2021

Oops. Conflicts :-/

@choldgraf
Copy link
Member

choldgraf commented Oct 16, 2021

OK I think it looks correct now, but to give ourselves some time I just cut a release of the theme, and that should give a bit of extra time to make sure nothing is unexpectedly broken.

many thanks @ocefpaf for the contribution :-)

@choldgraf choldgraf changed the title Update pydata sphinx theme UPDATE: pydata sphinx theme v0.7.1 Oct 16, 2021
@choldgraf choldgraf merged commit e02177a into executablebooks:master Oct 16, 2021
@welcome
Copy link

welcome bot commented Oct 16, 2021

Congrats on your first merged pull request in this project! 🎉
congrats

Thank you for contributing, we are very proud of you! ❤️

@ocefpaf ocefpaf deleted the update_pydata-sphinx-theme branch November 26, 2021 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants