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

Add "Open in Codepen" to link example #1693

Merged
merged 5 commits into from
Dec 23, 2020
Merged

Add "Open in Codepen" to link example #1693

merged 5 commits into from
Dec 23, 2020

Conversation

spectranaut
Copy link
Contributor

@spectranaut spectranaut commented Dec 18, 2020

In order to add the "Open in Codepen" button to this example, I did the following:

  1. I update the PNG to an SVG so the two image link examples use the same SVG image
  2. For the example that uses "css :before", I encoded the SVG into a url in the CSS to handle the fact that relative paths in CSS do not work when the code is sent to codepen

Preview

Copy link
Contributor

@carmacleod carmacleod left a comment

Choose a reason for hiding this comment

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

Will you be adding the Open in CodePen button in a different PR?

@spectranaut
Copy link
Contributor Author

Whoops! Added @carmacleod :)

Copy link
Contributor

@carmacleod carmacleod left a comment

Choose a reason for hiding this comment

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

Looks good!
Thanks @spectranaut !

Copy link
Contributor

@mcking65 mcking65 left a comment

Choose a reason for hiding this comment

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

@spectranaut just one question.

Btw, thank you for all this awesome work on the images so they work in codepen.

@@ -55,6 +55,12 @@ table.data.attributes tbody td {
hyphens: manual;
}

/* We need to override the top margin for the link example */
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have to make a change for a specific example in the core.css?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of the other examples follow the same layout -- there is a header which describes the example. If there is just on example, it says "Example". If there are multiple examples on the page, there is a header for each example (something like: "Example 1: Simple List of Links"). The open in codepen button is added automatically as a sibling element to this header.

In the case of the links, though, the three examples are in a table. So the styling has to be slightly different to look good.. I'm not sure another place to put it, I could put it in the link example pages css but that css is ONLY for the example itself not the surrounding page.

Copy link
Contributor

Choose a reason for hiding this comment

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

@spectranaut wrote:

All of the other examples follow the same layout -- there is a header which describes the example. If there is just on example, it says "Example". If there are multiple examples on the page, there is a header for each example (something like: "Example 1: Simple List of Links"). The open in codepen button is added automatically as a sibling element to this header.

In the case of the links, though, the three examples are in a table. So the styling has to be slightly different to look good..

Oh, I didn't realize the styling was that different. Link was one of the very early examples. And, due to its simplicity, it has had almost no attention over the years.

I'm not sure another place to put it, I could put it in the link example pages css but that css is ONLY for the example itself not the surrounding page.

It makes sense to be in core. We might want to revisit the page design. That is likely to happen naturally as part of the redesign project, so I guess we don't need a separate issue for it.

Copy link
Contributor

@carmacleod carmacleod Dec 23, 2020

Choose a reason for hiding this comment

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

@mcking65 @spectranaut
Another possibility is to put the whole table with all 3 link examples into a single codepen instead of having 3 separate codepens.

@mcking65 mcking65 added enhancement Any addition or improvement that doesn't fix a code bug or prose inaccuracy Example Page Related to a page containing an example implementation of a pattern labels Dec 23, 2020
@mcking65 mcking65 added this to the 1.2 Release 1 milestone Dec 23, 2020
@mcking65 mcking65 merged commit 2c38b7f into master Dec 23, 2020
@mcking65 mcking65 deleted the codepen-link-example branch December 23, 2020 01:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Any addition or improvement that doesn't fix a code bug or prose inaccuracy Example Page Related to a page containing an example implementation of a pattern
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants