webdev-live show sessions #2878
Conversation
|
Deploy preview for web-dev-staging ready! Built with commit 988c493 |
| } | ||
|
|
||
| table.w-event-schedule { | ||
| // TODO(robdodson): The generic table has `min-width: 512px`, which extends over the side of |
There was a problem hiding this comment.
@samthor I think we usually enforce tables be wrapped in an outer <div class="w-table-wrapper"> so they can overflow-x properly on small devices. But we can also see about removing the mind-width.
There was a problem hiding this comment.
Thanks for the clarification. In this case, there's no reason to let it extend—although cramped, the visual design works well at 320px.
| @@ -49,6 +50,7 @@ const YouTube = require(`./${componentsDir}/YouTube`); | |||
|
|
|||
| const collectionsDir = 'src/site/_collections'; | |||
| const authors = require(`./${collectionsDir}/authors`); | |||
| const eventSchedule = require(`./${collectionsDir}/event-schedule`); | |||
There was a problem hiding this comment.
This doesn't need to be an eleventy collection. You can access items in the _data folder inside of any page template by using its directory name. For example, the event.json file is available to all pages as {{ event }}.
| module.exports = (collections) => { | ||
| const schedule = collections.eventSchedule; | ||
|
|
||
| // TODO(MichaelSolati): "Author" needs a post but doesn't use it, we pass an empty object. |
There was a problem hiding this comment.
As an alternative you could use the w-author HTML / classes directly. The shortcode is syntactic sugar but if you're having to pass it bogus arguments that feels off.
There was a problem hiding this comment.
The Author shortcode is actually quite convenient here, I really like it.
This comment (cc @MichaelSolati) was mostly observing that I think the Author shortcode has post left over from an older time, it's just not used.
Another bit of feedback might be that it could be nice to automatically (yet again I'm confused about Eleventy's data) load the "author" information from the id. Since I've had to do a bunch of work to load it up correctly.
| // const prettyDate = require('../../_filters/pretty-date'); | ||
| // const stripLanguage = require('../../_filters/strip-language'); | ||
| // const md = require('../../_filters/md'); | ||
| // const constants = require('../../_utils/constants'); | ||
| // const getSrcsetRange = require('../../_utils/get-srcset-range'); | ||
| // const postTags = require('../../_data/postTags'); |
There was a problem hiding this comment.
should these be deleted?
There was a problem hiding this comment.
Yup. I did it in the web-event-time PR.
| @@ -0,0 +1,46 @@ | |||
| /* | |||
There was a problem hiding this comment.
You may want to move this file up to the _data directory. Alternatively, since stuff in _data is in the global namespace, it might make sense to make these directory data files that live in the /live/ directory.
Since they're not using the eleventy collections object, it feels a little off to put them in the collections directory.
There was a problem hiding this comment.
Thanks for the suggestion. This was a bit ambitious and I'll move it to "_data" on Monday.
| </div> | ||
| </web-tabs> | ||
|
|
||
| {% EventTable collections %} |
There was a problem hiding this comment.
I'll fix on Monday.
| }; | ||
|
|
||
| return html` | ||
| <web-tabs class="w-event-tabs"> |
There was a problem hiding this comment.
It seems like the tabs component has a required label attribute. Otherwise it'll set an aria-label on itself of undefined. Not sure why it was written that way, but would you mind adding a label="schedule" here? That way it'll announce "selected tab, 1 of 3, schedule, tab group".
There was a problem hiding this comment.
How odd. I'll fix on Monday.
| @@ -213,6 +216,7 @@ module.exports = function(config) { | |||
| config.addPairedShortcode('CompareCaption', CompareCaption); | |||
| config.addPairedShortcode('Details', Details); | |||
| config.addPairedShortcode('DetailsSummary', DetailsSummary); | |||
| config.addShortcode('EventTable', EventTable); | |||
There was a problem hiding this comment.
Would you mind adding a comment that we should remove this shortcode if we take down the live page, or if the live page no longer uses it. I think the post-event mocks don't use it so at that point it'd just be dead weight.
* collection and display sessions * disable property-sort-order
Shows sessions for web.dev/live.
Disables property-sort-order as it doesn't play nice with CSS Variables, see issue (which has been going for ~2 years, has a PR, and seems to be being ignored).
Adds just a couple of sessions for now. We can fill them in.