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

Tweek css for sphinx_prompt #5281

Merged
merged 4 commits into from
Feb 26, 2019
Merged

Tweek css for sphinx_prompt #5281

merged 4 commits into from
Feb 26, 2019

Conversation

ovc
Copy link
Contributor

@ovc ovc commented Feb 13, 2019

Fix for sphinx_prompt CSS, if you call sphinx_prompt with different prompt sign after previous call.

Copy link
Member

@dojutsu-user dojutsu-user 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 PR.
But, it is unclear to me that what problem does it addresses.
Can you also please post a screenshot of the changes.

@ovc
Copy link
Contributor Author

ovc commented Feb 13, 2019

If you will use another symbol in a prompt, a function of the prompt will generate another span class with next number, and after all your custom CSS will break the text size.

cat ./sphinx_prompt_css.css
...
pre.highlight span.prompt1 {
font-size: 12px;
line-height: 1.4;
}

cat ./sphinx_page_with_prompt.rst

capture1

The second call of the prompt call break the text (size is different), because of :

prompt

Thank you for your review!

@stsewd
Copy link
Member

stsewd commented Feb 13, 2019

Thanks for the explanation, this looks like only affect to our docs, but looks like a good fix

@stsewd
Copy link
Member

stsewd commented Feb 13, 2019

Also, can you please rebase/merge master? The test failure was already fixed there

@ovc
Copy link
Contributor Author

ovc commented Feb 14, 2019

Also, can you please rebase/merge master? The test failure was already fixed there

Can you please explain better, what I need to do? Thank you!

@stsewd
Copy link
Member

stsewd commented Feb 14, 2019

@ovc in your branch execute

git checkout master
git pull origin
git checkout <your-branch>
git merge master
git push <your-remote> <your-branch>

@humitos humitos added the PR: work in progress Pull request is not ready for full review label Feb 20, 2019
@ovc
Copy link
Contributor Author

ovc commented Feb 23, 2019

@stsewd my changes was already merged in master. I already done another commit. I'm sorry, but I can't understand what I need to do with this trevis error.

@dojutsu-user
Copy link
Member

dojutsu-user commented Feb 23, 2019

@ovc
It is considered a good practice to create a separate branch for each PR.
Since you have made a PR from the master branch, you have two options.

  1. Pull updated changes in your master branch and merge them, add, commit and push.
    You can find the instructions on how to sync a forked repo here : https://help.github.com/en/articles/syncing-a-fork
  2. Close this PR and make another PR from a separate branch.
    If you choose this option, make sure that you sync your repo with the latest code updates.

When you are in a separate branch, you can follow the instructions above to merge master branch into your branch.
Thanks

@ovc
Copy link
Contributor Author

ovc commented Feb 23, 2019

@dojutsu-user thank you for your explanation, I merged the changes from upstream/master for now.

Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

Makes sense. 👍

@ericholscher ericholscher merged commit 9006697 into readthedocs:master Feb 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: work in progress Pull request is not ready for full review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants