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

STY Adjusts layout for Authors #15095

Merged
merged 6 commits into from
Oct 2, 2019
Merged

Conversation

cmarmo
Copy link
Contributor

@cmarmo cmarmo commented Sep 26, 2019

What does this implement/fix? Explain your changes.

This PR suggests a new layout for the author table

Any other comments?

I have manually modified authors.rst because I have no rights to run generate_authors_table_py.
Though, script changes are included (it's like I took a bet... let me know...)

@amueller
Copy link
Member

please fix the linting errors, as they prevent the page from being rendered in CI.

@jnothman
Copy link
Member

jnothman commented Sep 27, 2019 via email

@cmarmo
Copy link
Contributor Author

cmarmo commented Sep 27, 2019

With the images centred, should the text be centred?

On the web left-aligned text is in general recommended (at least for left to right writing languages). This avoids strange behaviors when resizing.... but if you prefer centred... I have no feelings on that.

doc/themes/scikit-learn-modern/static/css/theme.css Outdated Show resolved Hide resolved
doc/themes/scikit-learn-modern/static/css/theme.css Outdated Show resolved Hide resolved
build_tools/generate_authors_table.py Outdated Show resolved Hide resolved
doc/authors.rst Outdated Show resolved Hide resolved
build_tools/generate_authors_table.py Outdated Show resolved Hide resolved
@thomasjpfan thomasjpfan changed the title Authors Adjusts layout for Authors Sep 28, 2019
Copy link
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

I fixed some small issues in the script and reran it and pushed the result of the execution.

LGTM. Thanks for the improvement @cmarmo. I like the new responsive layout.

Copy link
Member

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

Thank you @cmarmo and @ogrisel !

@thomasjpfan thomasjpfan changed the title Adjusts layout for Authors STY Adjusts layout for Authors Oct 2, 2019
@thomasjpfan thomasjpfan merged commit 6127b4e into scikit-learn:master Oct 2, 2019
@cmarmo cmarmo deleted the authors branch October 7, 2019 09:22
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.

5 participants