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

docs(aio): image sweep #16609

Merged
merged 8 commits into from May 9, 2017
Merged

Conversation

petebacondarwin
Copy link
Member

@petebacondarwin petebacondarwin commented May 6, 2017

All the images in the guides have been checked and tidied up.
Fixes #16396

@wardbell : The following should become part of the "Authors Styleguide":

Using Images

It is best to use HTML to declare images in the docs. Do not use the markdown ![...](...)
shorthand.
The HTML to use is an <img src="..." alt="..."> tag. You must supply a src attribute that is relative to the base path; and you should provide an alt attribute describing the image for accessibility. Note that Image tags do not have a closing tag.

Image Size

The doc generation process will read the image dimensions and automatically add width and
height attributes to the img tag. If you want to control the size of the image then you should
supply your own width and height images.

Images should not be wider than 700px otherwise they may overflow the width of the document
in certain viewports. If you wish to have a larger image then provide a link to the actual image that
the user can click on to see the larger images separately.

Image Formatting

There are three types of images that you can put in docs: inline, floating and standalone.

Inline Images

To create an inline image simply place the img tag in the content where you want the image to appear. E.g.

The image here <img src="..." alt="..."> is visible inline in the text.

Floating images

You can cause an image to float to the left or right of the text by applying the class="left" or class="right" attributes respectively. E.g.

<img src="..." alt="..." class="left">
This text will wrap around the to the right of this floating image.

All headings and code-examples will automatically clear a floating image.
If you need to force a piece of text to clear a floating image then you can use the following snippet:

<br class="clear">

Finally, if you have floating images inside alerts or sub-sections then it is a good idea to apply the clear-fix class to the div to ensure that the image doesn't overflow its container. E.g.

<div class="l-sub-section clear-fix">

  <img class="right" src="..." alt="...">
  Some **markdown** formatted text.

</div>

Standalone images

Some images should stand alone from the text. You do this by wrapping the img tag in a figure
tag. This causes the image to get additional styling, such as a rounded border and shadow. The text will not flow around this image but will stop before the image and start again afterword. E.g.

Some paragraph preceding the image.

<figure>
  <img src="" alt="">
</figure>

(In the future we may also support styling for adding a caption to a standalone image.)

@mary-poppins
Copy link

The angular.io preview for c13cd25 is available here.

@mary-poppins
Copy link

The angular.io preview for 8294ad3 is available here.

@mary-poppins
Copy link

The angular.io preview for 025c329 is available here.

@mary-poppins
Copy link

The angular.io preview for 9475ec7 is available here.

@mary-poppins
Copy link

The angular.io preview for 383ad5e is available here.

@mary-poppins
Copy link

The angular.io preview for ff25d22 is available here.

@mary-poppins
Copy link

The angular.io preview for f487edb is available here.

@petebacondarwin petebacondarwin changed the title aio": image sweep (WIP) docs(aio): image sweep May 9, 2017
@petebacondarwin petebacondarwin added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: WIP labels May 9, 2017
float: none !important;
}
}
.content {
Copy link
Member Author

Choose a reason for hiding this comment

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

We only want this formatting of images within the content of documents, not the aio-shell in general.

@media (max-width: 1300px) {
max-width: 100%;
height: auto;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

If the screen is wider than 1300px then we want to use the width and height values provided in the HTML to ensure that the page is laid out correctly while the images are still loading.

Copy link
Member Author

Choose a reason for hiding this comment

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

But then as the screen narrows the images will be constrained by the max-width rule, which means that we need to switch to using height: auto to ensure that the proportions of the image do not change.

@mary-poppins
Copy link

The angular.io preview for 151d795 is available here.

@mary-poppins
Copy link

The angular.io preview for 68263a1 is available here.

Copy link
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

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

LGTM (with a few minor comments/questions).

<figure class='image-display'>
<img src="generated/images/guide/aot-compiler/toh-pt6-bundle.png" alt="toh-pt6-bundle"></img>
<figure>
<img src="generated/images/guide/aot-compiler/toh-pt6-bundle.png" alt="toh-pt6-bundle" width="700">
Copy link
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Member Author

Choose a reason for hiding this comment

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

This raw image is super big. A width of 700px is about all you can manage on a screen that is at the breakpoint of 1300px before the max-width: 100% kicks in.

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we resize the image then (and save the cost of downloading it)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes but that was outside of my remit :-P
I'll create an issue to track it.

Copy link
Member Author

Choose a reason for hiding this comment

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

See #16678

<figure class='image-display'>
<img src="generated/images/guide/upgrade/injectors.png" alt="The two injectors in a hybrid application" width="700"></img>
<figure>
<img src="generated/images/guide/upgrade/injectors.png" alt="The two injectors in a hybrid application" width="700" height="262">
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need manualy width/height here?

Copy link
Member Author

Choose a reason for hiding this comment

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

The actual image is too large for the width of the page at the breakpoint where we switch to max-width: 100% and height: auto. 700 is the biggest width that works at this screen width.

<figure class='image-display'>
<img src="generated/images/guide/reactive-forms/json-output.png" width="400px" alt="JSON output"></img>
<figure>
<img src="generated/images/guide/reactive-forms/json-output.png" width="400" height="133" alt="JSON output">
Copy link
Member

Choose a reason for hiding this comment

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

How did you determine these heights. This one looks "sqeezed" for example?

Copy link
Member

Choose a reason for hiding this comment

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

I was going to comment this one too.

Copy link
Member Author

Choose a reason for hiding this comment

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

For each image I set the width to the suggested width and then checked that the browser resized the height to. I may have got it wrong in this case. It should have been 176.


&.right {
float: right;
margin-left: 20px;
Copy link
Member

Choose a reason for hiding this comment

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

This left margin doesn't look so goodon narrow screens (where max-width: 100%).

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call, we need to lose the floating when narrow, I guess.

Copy link
Member Author

Choose a reason for hiding this comment

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

screen shot 2017-05-09 at 23 08 16


<figure>
<img src="generated/images/guide/architecture/parent-child-binding.png" alt="Parent/Child binding" style="float:left; width:300px; margin-left:-40px;margin-right:10px"></img>
<img src="generated/images/guide/architecture/parent-child-binding.png" alt="Parent/Child binding" width="400">
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I think the idea was to make this image the same size as the previous one (which looks better). This behavior is not retained here.

Copy link
Member Author

Choose a reason for hiding this comment

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

agreed - will fix

Copy link
Member Author

Choose a reason for hiding this comment

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

screen shot 2017-05-09 at 23 07 35

@@ -42,4 +42,5 @@ module.exports = new Package('angular.io', [gitPackage, apiPackage, contentPacka
return (existsSync(resolve(SRC_PATH, url)));
}
});
checkAnchorLinksProcessor.pathVariants = ['', '/', '.html', '/index.html', '#top-of-page'];
Copy link
Member

Choose a reason for hiding this comment

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

What is .html for?

Copy link
Member Author

Choose a reason for hiding this comment

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

The .html is only there because the defaults for that processor were: ['', '/', '.html', '/index.html'], so I am really just adding one new one. :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

you'd have never known if I'd just used .push('#....').

Copy link
Member

Choose a reason for hiding this comment

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

Don't be so sure 😉

@mary-poppins
Copy link

The angular.io preview for f66dc2e is available here.

Copy link
Contributor

@wardbell wardbell left a comment

Choose a reason for hiding this comment

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

Same question. I do like baking #top-of-page though.

Otherwise, lgtm

<figure class='image-display'>
<img src="generated/images/guide/reactive-forms/json-output.png" width="400px" alt="JSON output"></img>
<figure>
<img src="generated/images/guide/reactive-forms/json-output.png" width="400" height="133" alt="JSON output">
Copy link
Member

Choose a reason for hiding this comment

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

I was going to comment this one too.

@@ -29,7 +29,7 @@ See the <live-example name="set-document-title"></live-example>.
</td>

<td>
<img src='generated/images/plunker/plunker-switch-to-editor-button.png' width="200px" height="70px" alt="pop out the window" align="right"></img> <br></br> <img src='generated/images/plunker/plunker-separate-window-button.png' width="200px" height="47px" alt="pop out the window" align="right"></img>
<img src='generated/images/plunker/plunker-switch-to-editor-button.png' width="200" height="70" alt="pop out the window" align="right"> <br></br> <img src='generated/images/plunker/plunker-separate-window-button.png' width="200" height="47" alt="pop out the window" align="right">
Copy link
Member

Choose a reason for hiding this comment

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

This in narrow mode is almost invisible.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, that is because the image is in a td and it's width gets set to max-width: 100% but that it 100% of the td rather than the page. We might need some additional styling to cope with this situation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually using a table is not right here. It is much better to use a subsection and floating images. I'll fix up.
screen shot 2017-05-09 at 23 04 14

@Foxandxss
Copy link
Member

The embedded plunker preview images are suuuuuuuuuper big.

@petebacondarwin
Copy link
Member Author

petebacondarwin commented May 9, 2017

I fixed the embedded plunker images.

screen shot 2017-05-09 at 22 57 51

The css rules for `img.right` and `img.left` allow authors easy
access to floating an image on the left or right, respectively.

The `.image-display` rule which was always found on a figure
has been simplified so that all figures have this styling. It is very
unlikely that a figure will be used outside the content area; and
at this time it seems like `figure` is as good an indicator that we
want this kind of styling as anything.

Now that images are all tagged with width and height values, we cannot
assume to modify these dimensions via CSS as it can cause the image to
lose its correct proportions.  Until we find a better solition we must set
`height` to `auto` when the screen width is below 1300px to ensure that
these images maintain their proportions as they get shrunk to fit.
Previously the negative margin on the code headings were causing
floated images to overlay the start of a code block. Now all code block
successfully clear all floated elements.
Previously, the guides have a lot of inline image styling and unnecessary
use of the `image-display` css class.
Images over 700px are problematic for guide docs, so those have been given
specific widths and associated heights.
The `#toc` anchor does not work when the page is
wide enough that the TOC is floating to the side.
Since the `#top-of-page` is outside the rendered docs
the `checkAnchorLinks` processor doesn't find them
as valid targets for links.
Adding them as a `pathVariant` solves this problem
but will still catch links to docs that do not actually exist.
This made them look too big, generally. Leaving them with no size means
that they will look reasonable in large viewports and switch to 100% width
in narrow viewports.
@petebacondarwin
Copy link
Member Author

I think this is all fixed up @Foxandxss and @gkalpak PTAL

@mary-poppins
Copy link

The angular.io preview for 84b16a1 is available here.

@mary-poppins
Copy link

The angular.io preview for 1802c5c is available here.

@Foxandxss
Copy link
Member

Works for me pete.

@petebacondarwin petebacondarwin added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels May 9, 2017
@petebacondarwin petebacondarwin moved this from REVIEW to MERGE in docs-infra May 9, 2017
@jasonaden jasonaden merged commit 9e661e5 into angular:master May 9, 2017
@petebacondarwin petebacondarwin removed this from MERGE in docs-infra May 9, 2017
@petebacondarwin petebacondarwin deleted the aio-image-sweep branch May 9, 2017 22:58
asnowwolf pushed a commit to asnowwolf/angular that referenced this pull request Aug 11, 2017
* fix(aio): allow code blocks to clear floated images

Previously the negative margin on the code headings were causing
floated images to overlay the start of a code block. Now all code block
successfully clear all floated elements.

* feat(aio): add a `.clear` class for clearing floating images

* fix(aio): tidy up image styles

The css rules for `img.right` and `img.left` allow authors easy
access to floating an image on the left or right, respectively.

The `.image-display` rule which was always found on a figure
has been simplified so that all figures have this styling. It is very
unlikely that a figure will be used outside the content area; and
at this time it seems like `figure` is as good an indicator that we
want this kind of styling as anything.

Now that images are all tagged with width and height values, we cannot
assume to modify these dimensions via CSS as it can cause the image to
lose its correct proportions.  Until we find a better solition we must set
`height` to `auto` when the screen width is below 1300px to ensure that
these images maintain their proportions as they get shrunk to fit.

* docs(aio): general tidy up of image HTML in guides

Previously, the guides have a lot of inline image styling and unnecessary
use of the `image-display` css class.
Images over 700px are problematic for guide docs, so those have been given
specific widths and associated heights.

* docs(aio): use correct anchor for "back to the top" link

The `#toc` anchor does not work when the page is
wide enough that the TOC is floating to the side.

* build(aio): add `#top-of-page` to path variants for link checking

Since the `#top-of-page` is outside the rendered docs
the `checkAnchorLinks` processor doesn't find them
as valid targets for links.
Adding them as a `pathVariant` solves this problem
but will still catch links to docs that do not actually exist.

* fix(aio): ensure that headings clear floated images

* fix(aio): do not force live-example embedded image to 100% size

This made them look too big, generally. Leaving them with no size means
that they will look reasonable in large viewports and switch to 100% width
in narrow viewports.
juleskremer pushed a commit to juleskremer/angular that referenced this pull request Aug 28, 2017
* fix(aio): allow code blocks to clear floated images

Previously the negative margin on the code headings were causing
floated images to overlay the start of a code block. Now all code block
successfully clear all floated elements.

* feat(aio): add a `.clear` class for clearing floating images

* fix(aio): tidy up image styles

The css rules for `img.right` and `img.left` allow authors easy
access to floating an image on the left or right, respectively.

The `.image-display` rule which was always found on a figure
has been simplified so that all figures have this styling. It is very
unlikely that a figure will be used outside the content area; and
at this time it seems like `figure` is as good an indicator that we
want this kind of styling as anything.

Now that images are all tagged with width and height values, we cannot
assume to modify these dimensions via CSS as it can cause the image to
lose its correct proportions.  Until we find a better solition we must set
`height` to `auto` when the screen width is below 1300px to ensure that
these images maintain their proportions as they get shrunk to fit.

* docs(aio): general tidy up of image HTML in guides

Previously, the guides have a lot of inline image styling and unnecessary
use of the `image-display` css class.
Images over 700px are problematic for guide docs, so those have been given
specific widths and associated heights.

* docs(aio): use correct anchor for "back to the top" link

The `#toc` anchor does not work when the page is
wide enough that the TOC is floating to the side.

* build(aio): add `#top-of-page` to path variants for link checking

Since the `#top-of-page` is outside the rendered docs
the `checkAnchorLinks` processor doesn't find them
as valid targets for links.
Adding them as a `pathVariant` solves this problem
but will still catch links to docs that do not actually exist.

* fix(aio): ensure that headings clear floated images

* fix(aio): do not force live-example embedded image to 100% size

This made them look too big, generally. Leaving them with no size means
that they will look reasonable in large viewports and switch to 100% width
in narrow viewports.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aio: fix right-aligned animation guide images that overlap text
7 participants