It doesn't look like the SASS files are used at all. Is that right?

If so, I don't see any reason to keep them.

If they are being used, what are you using to compile them? It'd be nice to have something standard, like grunt, webpack, etc.

There are two menu areas output in the header. I think many camps will only want one, so the secondary one should probably only be displayed if there's a menu assigned to it.

Hi,

I am working on a WordPress site using the CampSite 2017 theme.

First of all, thanks to everyone involved in making this awesome theme!

While setting up the site, I spotted some issues, and wondered where they can be discussed, pull requests made, etc.

Is this repository a good place? Is it the current and maintained version?

It would be great if there was a README that would clarify those questions, and would point to the best place where fellow developers can contribute.

Is it possible to get the Sketch/Photoshop export of the design? Awesome work btw 🎉 🍾

I don't have time to look closely right now, but it seems like the social and top menus that are references in icon-functions.php aren't registered locations.

It seems like we should register social, and then maybe rename top to primary ?

It'd be nice to have a non-empty screenshot.png. A shot of 2017.europe.wordcamp.org might make sense.

• functions.php:46 - Combine the two register_nav_menus() calls into one
• functions.php:379 - Since this isn't a child theme, why use get_parent_theme_file_path() instead of get_theme_file_path()?
• comments.php:31 - gettext call containing placeholders is not accompanied by "translators:" comment.
• comments.php:31 - Singular placeholder is missing, which is needed for some languages. https://codex.wordpress.org/I18n_for_WordPress_Developers#Plurals
• content-day-of.php:3 - has wrong template name in file header
• If there are functions that are copied over from something other than _s (e.g. Twenty Seventeen), that should be noted in the doc block for the function, including the version of the theme it's copied from.

It seems like any speaker-specific content would work better as a plugin, so that it'll work for all themes, rather than us having to re-create it in each theme.

The wc-post-types plugin already adds links to sessions, slides, and videos, too, so I'm not sure that's necessary.

Am I missing anything?

Since all the widget areas use the same values for before_widget, after_widget, before_title, and after_title, perhaps rework the campsite_2017_widgets_init() function to be DRYer and use less repetitive code. For example:

$widget_areas = array(
	array(
		'name'          => esc_html__( 'Primary Sidebar', 'wordcamporg' ),
		'id'            => 'sidebar-1',
		'description'   => esc_html__( 'Main Widgets Sidebar. Shows up in all pages.', 'wordcamporg' ),
	),
	array(
		'name'          => esc_html__( 'Secondary Sidebar', 'wordcamporg' ),
		'id'            => 'sidebar-2',
		'description'   => esc_html__( 'Secondary Widgets Sidebar - shows up in all pages after the Primary Sidebar block.', 'wordcamporg' ),
	),
	array(
		'name'          => esc_html__( 'Before Content (All pages except homepage)', 'wordcamporg' ),
		'id'            => 'before-content-1',
		'description'   => esc_html__( 'Will show a widgets area, inside the #content block, before all the content, in all pages except the homepage.', 'wordcamporg' ),
	),
	// etc...
);

foreach ( $widget_areas as $widget_area ) {
	$args = array_merge( array(
		'before_widget' => '<section id="%1$s" class="widget %2$s">',
		'after_widget'  => '</section>',
		'before_title'  => '<h2 class="widget-title">',
		'after_title'   => '</h2>',
	), $widget_area );

	register_sidebar( $args );
}

$unique_id should be wrapped with esc_attr. Even though it's generated solely by another PHP function, and thus "safe", the way it's generated could change in the future so it's better to not have to also remember to escape it at that point in time.

The early returns at the beginning of this function include translatable strings that would never be shown (I think) on WordCamp.org, since they are essentially error messages for incorrect usage of the PHP function. My feeling is that these should probably be removed to reduce the number of strings that need to be translated.

If they stay in, note that they are using the wrong text domain.

I'm curious why the _s navigation.js was replaced with twentyseveteen's. I think it'd be good to document that.

Today I got the feedback for our site https://2019.vienna.wordcamp.org/ that if you toggle the menu open on mobile and coincidently your screen height makes it so that the menu toggle button aligns with the bottom of the screen you have hardly any feedback that anything happened.

I was wondering if that is a known thing and/or intentional (Not sure how great it is to scroll things around with regards to a11y) or something that could be improved.

WordCamp.org supports HTTP2, so spriting images doesn't significantly help load times. Without the performance gains, I don't think the added complexity and maintenance is worth it.

These seem to be identical other than a class on the primary container. Are they both needed?

It doesn't look like the responsive menu toggling is working, unless I've messed something up locally :)

It seems like maybe a difference between the toggled and toggled-on attributes in _s vs TwentySeventeen, but I haven't looked closely.

It looks like the Before Content (Homepage) widgets show up on all pages except the homepage, because of the conditions in sidebar-before-content.php.