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

reformat - move paragraph direction and image size from style attribute to "classic" attribute #39

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ophiry
Copy link

@ophiry ophiry commented Sep 14, 2020

re: #37

@@ -4,23 +4,23 @@
"cell_type": "markdown",
"metadata": {},
"source": [
"<img src=\"images/logo.jpg\" style=\"display: block; margin-left: auto; margin-right: auto;\" alt=\"לוגו של מיזם לימוד הפייתון. נחש מצויר בצבעי צהוב וכחול, הנע בין האותיות של שם הקורס: לומדים פייתון. הסלוגן המופיע מעל לשם הקורס הוא מיזם חינמי ללימוד תכנות בעברית.\">"
"<img alt=\"לוגו של מיזם לימוד הפייתון. נחש מצויר בצבעי צהוב וכחול, הנע בין האותיות של שם הקורס: לומדים פייתון. הסלוגן המופיע מעל לשם הקורס הוא מיזם חינמי ללימוד תכנות בעברית.\" src=\"images/logo.jpg\" style=\"display: block; margin-left: auto; margin-right: auto;\"/>"
Copy link
Member

Choose a reason for hiding this comment

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

If it is OK on your side, I'd prefer not to move around attributes in places where it doesn't make a difference :)

Copy link
Author

Choose a reason for hiding this comment

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

I used a html parser (BeautifulSoup) to do the reformatting - there's no direct control on all the details of the generated html

"לפעמים נרצה לצרף דברי הסבר לקוד שכתבנו.<br>\n",
"כדי להוסיף הסברים לצד הקוד נשתמש בתו סולמית, ומייד אחריה נכתוב את ההסברים שרצינו לצרף.<br>\n",
"<p dir=\"rtl\" style=\"text-align: right; direction: rtl; float: right; clear: both;\">\n",
"לפעמים נרצה לצרף דברי הסבר לקוד שכתבנו.<br/>\n",
Copy link
Member

Choose a reason for hiding this comment

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

Let's not add closing tags to our brs, we're on HTML5 :)

"<li><code>\"_)(*&^%\"</code></li>\n",
"<li><code>\"_)('*&^%\"</code></li>\n",
" <li><code>\"_)(\"*&^%\"</code></li>\n",
"<li><code>\"_)(*&amp;^%\"</code></li>\n",
Copy link
Member

Choose a reason for hiding this comment

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

Is this behavior required?

@smith-kyle
Copy link

I'm a little late to the party, but I noticed you all aren't using a notebook review tool and wanted to invite you to review this pull request with GitNotebooks: https://gitnotebooks.com/PythonFreeCourse/Notebooks/pull/39

It lets you do things like comment on rendered markdown and code cells, so might be an easy win for your PR reviews.

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.

None yet

3 participants