Skip to content

DOC Adjust layout for 'testimonials' page #15145

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
Merged
merged 4 commits into from
Oct 15, 2019
Merged

DOC Adjust layout for 'testimonials' page #15145

merged 4 commits into from
Oct 15, 2019

Conversation

cmarmo
Copy link
Contributor

@cmarmo cmarmo commented Oct 7, 2019

What does this implement/fix? Explain your changes.

This PR suggests a new and more responsive layout for the "Testimonials" page, addressing the proposition made here

ogrisel
ogrisel reviewed Oct 7, 2019
View reviewed changes
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.

Thanks for the responsive layout. Could you please try to make the testimonial headers left-aligned with the following paragraph? At the moment there is some additional padding/indentation for the header.

thomasjpfan
thomasjpfan reviewed Oct 8, 2019
View reviewed changes
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.

I think this layout will still work without the sk-testimonial-div-box:

<div class="sk-testimonial">...
<p>...
<div class="testtimonial-author">...
<a>...
</div>

@amueller
Copy link
Member

amueller commented Oct 8, 2019

How about instead we remove this page?

@cmarmo
Copy link
Contributor Author

cmarmo commented Oct 9, 2019

How about instead we remove this page?

Hello, I'm stopping any work on this PR until this question answered... hope you understand... :)

@ogrisel
Copy link
Member

ogrisel commented Oct 9, 2019

How about instead we remove this page?

Why? Do you think the testimonials are too old? Or this page is leading to incentivize abuse for SEO/marketing purpose?

I still think it's useful to give datascience students, journalists, funding agencies and business managers some specific example of business (and scientific) applications of machine learning tools like scikit-learn.

@cmarmo
Copy link
Contributor Author

cmarmo commented Oct 14, 2019

I've synchronized to master.

I think this layout will still work without the sk-testimonial-div-box:

<div class="sk-testimonial">...
<p>...
<div class="testtimonial-author">...
<a>...
</div>

I'm not sure to understand... I need the flex-container and multiple flex-boxes in order to generate flexible reordering of different div (https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Flexible_Box_Layout/Basic_Concepts_of_Flexbox)....

@thomasjpfan
Copy link
Member

In a follow up PR, can we rename the sk-sponsor-div-box/sk-sponsor-div and sk-testimonial-div-box/sk-testimonial-div? Currently it is unclear which one is the child and parent.

@thomasjpfan thomasjpfan merged commit a5f32da into scikit-learn:master Oct 15, 2019
@cmarmo cmarmo deleted the testimonialsPR branch October 16, 2019 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ogrisel ogrisel ogrisel approved these changes

@thomasjpfan thomasjpfan thomasjpfan 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.
4 participants
@cmarmo @amueller @ogrisel @thomasjpfan