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

Updating image references in hub page #1416

Merged
merged 13 commits into from Jan 18, 2017
Merged

Updating image references in hub page #1416

merged 13 commits into from Jan 18, 2017

Conversation

jasongroce
Copy link
Contributor

No description provided.

@jasongroce
Copy link
Contributor Author

#sign-off @mairaw This fixes an issue on the live site where the SVGs are being cut off on the Get Started panel. Can I get this merged and published at your convenience? Thanks!

@mairaw
Copy link
Contributor

mairaw commented Jan 17, 2017

There are some warnings on the build report. Have you seen those?

@mairaw
Copy link
Contributor

mairaw commented Jan 17, 2017

also @jasongroce, can we rename the files to all be lower caps?

@@ -128,7 +117,7 @@ ms.assetid:
<div class="card">
<div class="cardImageOuter">
<div class="cardImage">
<img src="./docs/images/hub/net-gs-3.svg" alt="" />
<img src="articles/images/hub/logo_NETframework.svg" alt="" />
Copy link
Contributor

Choose a reason for hiding this comment

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

the warnings are probably coming from these changed links. can you keep the links in the format "./docs/images/hub/" so we have clean builds?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I can update these. Oddly, the ad cards (at the top) use the articles/images/hub format so I thought there was some build system convention for this docset that needed to be followed. And yes, I can change them to lowercase.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see the issue. The hover image in the ad cards was pointing to the wrong place; I've made these consistent across the board.

ms.technology:
ms.suite:
ms.assetid:
description: Learn about .NET
Copy link
Contributor

Choose a reason for hiding this comment

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

we should probably add a more meaningful description if we're going to add this metadata.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're trying to include a meta description for SEO purposes, so this is new in the hub page template. If there's nothing better to put there for now, I can remove it.

<p><a href="/dotnet/articles/standard/components">.NET Architectural Concepts</a></p>
<p><a href="https://www.microsoft.com/net">Get Started</a></p>
<p><a href="/dotnet/articles/standard/#a-stroll-through-net">Tour of .NET</a></p>
<p><a href="/dotnet/articles/about/products">.NET Concepts</a></p>
Copy link
Contributor

Choose a reason for hiding this comment

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

why change this link? that article redirects to the old link: https://github.com/dotnet/docs/blob/master/docs/about/products.md

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll have to check these. It's likely that the version in /dotnet/docs was updated separately from the version in the production team's sandbox. I can revert these.

<p><a href="/dotnet/articles/standard/tour">Tour of .NET</a></p>
<p><a href="/dotnet/articles/standard/components">.NET Architectural Concepts</a></p>
<p><a href="https://www.microsoft.com/net">Get Started</a></p>
<p><a href="/dotnet/articles/standard/#a-stroll-through-net">Tour of .NET</a></p>
Copy link
Contributor

Choose a reason for hiding this comment

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

this new link doesn't exist. has someone asked to change the links in this section?

@jasongroce
Copy link
Contributor Author

jasongroce commented Jan 18, 2017

@mairaw This PR is now free of warnings. I believe I also reverted/updated all the links to what they were previously, and I've updated the version in the production repo to match. I'm submitting a separate update to fix the camel-cased images.

@jasongroce
Copy link
Contributor Author

#hold-off

@jasongroce
Copy link
Contributor Author

Reopening to rebuild to check build warnings on image links.

@jasongroce
Copy link
Contributor Author

#sign-off @mairaw I believe all issues are now addressed and the build report is clean. Let me know if you see anything amiss or have any concerns.

@mairaw
Copy link
Contributor

mairaw commented Jan 18, 2017

LGTM! Thanks @jasongroce!

@mairaw mairaw merged commit bfa92b9 into dotnet:master Jan 18, 2017
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