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

Fix an Alabaster ad positioning issue #3889

Merged

Conversation

@davidfischer
Copy link
Contributor

@davidfischer davidfischer commented Apr 2, 2018

screen shot 2018-04-02 at 12 27 43 pm

@davidfischer davidfischer requested a review from humitos Apr 2, 2018
@davidfischer davidfischer changed the title Fix an Alabaster positioning issue Fix an Alabaster ad positioning issue Apr 2, 2018
humitos
humitos approved these changes Apr 2, 2018
Copy link
Member

@humitos humitos left a comment

This fixes the original issue.

Although, isn't it still "too close" to the search box? Not sure if it's important, but these two elements are overlapping (on padding/margin)

captura de pantalla_2018-04-02_14-35-45

captura de pantalla_2018-04-02_14-36-12

I'm approving because the original issue is fixed. These comments are just to be considered.

Loading

@humitos
Copy link
Member

@humitos humitos commented Apr 2, 2018

Oh, I think another example without a white background looks too much beter and it's easier to see the changes :)

captura de pantalla_2018-04-02_14-39-18

Loading

@davidfischer
Copy link
Contributor Author

@davidfischer davidfischer commented Apr 2, 2018

I did some extra margin and a slightly better way to end the floating blocks.

screen shot 2018-04-02 at 2 35 56 pm

Loading

@humitos
Copy link
Member

@humitos humitos commented Apr 2, 2018

I like this new changes more. It feels nicer :)

Loading

@davidfischer davidfischer merged commit db111a4 into readthedocs:master Apr 2, 2018
1 check passed
Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants