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

Change css to fix problems with speakers and talks cards overflowing #2

Merged
merged 3 commits into from
Apr 20, 2018

Conversation

jpaddison3
Copy link
Contributor

No description provided.

Copy link
Contributor

@colophonemes colophonemes left a comment

Choose a reason for hiding this comment

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

A couple of TODOs I'd like to ask about, but then LGTM

@@ -37,7 +37,7 @@ block content
!= contents
if moment(endDate).add(14,'days').isAfter(moment(),'day') && contactEmail
.event-basic-info-contact
| For more information email
| For more information email 
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine, also you can use a string:

.thing
  | test text here#{' '}

Not necessarily nicer-looking, but makes it clearer what the intent is

@@ -142,6 +142,7 @@ function build(buildCount){


// START THE BUILD!
// TODO: what's this variable name doing?
Copy link
Contributor

Choose a reason for hiding this comment

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

Which variable?

Copy link
Contributor

@colophonemes colophonemes Apr 20, 2018

Choose a reason for hiding this comment

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

Ah, colophonemes... A hangover from a veeery long time ago and lots of copypasta 🍝

More specifically, this should be called metalsmith. It's possible to run a single chain of calls without the assignment by using the constructor:

Metalsmith(__dirname)
	.use(plugin1())
	.build()

However, we add some steps conditionally in production, so we have to assign to handle the breaks

const metalsmith = Metalsmith(__dirname)
	.use(plugin1())
if (NODE_ENV === 'production') {
	// do something expensive, like minifying HTML
	metalsmith
		.use(plugin2())
}
metalsmith.build()

@@ -42,13 +42,14 @@ block content
h2.speaker-talks-title Talks
.row
- var talkWidthClasses = ['col-sm-6','col-sm-offset-3','col-md-4', 'col-md-offset-4'];
//- TODO this is necessary for maximum height reasons, but throws of centering
Copy link
Contributor

Choose a reason for hiding this comment

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

What is necessary?

@jpaddison3 jpaddison3 merged commit 19b9237 into master Apr 20, 2018
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

2 participants