Add slide parts (and mark private methods) #11

mirisuzanne · 2023-12-28T06:45:08Z

Related Issue(s)

Reminder to add related issue(s), if available.

Clean up Public & Private API #14 - at least a partial fix?
Feature: Support for speaker notes #4 - with support for hiding either (but not both) the notes and the slide

Steps to test/reproduce

There's only one sample slide with notes near the end of the demo. But the control panel (command-k) has buttons for hiding one or the other.

I think this branch raises a few questions:

I called the three parts a slide-group, slide-frame, and slide-note. Can we come up with better names? slide-container and slide-canvas? Should it be slide-notes plural?
We can't provide shadow-dom default styles for anything nested inside a slotted element. Should we find a way around that, or stop providing default slide styles at all, or…?
Should this be documented as the primary use-case, or is it good that we allow a simpler all-children-are-slides setup? That may be a moot point if we remove the default styles.
If we do remove the shadow-dom styles, that maybe makes it more important that we provide a 'theme' people can apply.

netlify · 2023-12-28T06:45:12Z

✅ Deploy Preview for slide-deck ready!

Name	Link
🔨 Latest commit	`baa85a5`
🔍 Latest deploy log	https://app.netlify.com/sites/slide-deck/deploys/65a021bc9eaf350008220744
😎 Deploy Preview	https://deploy-preview-11--slide-deck.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

* main: Favicon

mirisuzanne · 2024-01-03T16:54:00Z

Discussion this morning:

wrapper: TBD… slide-container? slide-group? slide-item?
internals: slide-note and slide-canvas
remove shadow dom styles, and provide a light dom css file

mirisuzanne · 2024-01-04T18:28:06Z

@jamesnw @jerivas @stacyk I believe this is ready for review now

jamesnw

A few comments, and I am also wondering about the idea of a "part mode" or something like that- instead of specifying what is hidden, could we expose another view mode, where the options are View canvas, View Notes, and View Both? This more closely matches how I would think of it.

This overlaps in name and function with the existing view modes of grid and list. In List, should we allow notes only mode? I also am wondering if in List view, if Canvas + Notes is 100vh, so that both can be seen simultaneously?

jamesnw · 2024-01-04T19:11:25Z

slide-deck.js

  ];

  static attrToPropMap = {
    'key-control': 'keyControl',
    'follow-active': 'followActive',
    'full-screen': 'fullScreen',
    'slide-view': 'slideView',
+    'hide-parts': 'hideParts',


Since this only allows 1, should it just be hide-part?

jamesnw · 2024-01-04T19:15:07Z

slide-deck.js

@@ -98,57 +112,68 @@ class slideDeck extends HTMLElement {

  // dynamic
  store = {};
+  slides;
+  slideNotes;
+  slideCanvas;


I think slideNotes and slideCanvas should be private.

jamesnw · 2024-01-04T19:16:15Z

slide-deck.js

    this.controlPanel = this.querySelector(`[slot="control-panel"]`) ??
      this.shadowRoot.querySelector(`[part="control-panel"]`);
+    this.slides = this.querySelectorAll(':scope > :not([slot])');
+    this.slideNotes = this.querySelectorAll('[slide-note]');
+    this.slideCanvas = this.querySelectorAll('[slide-canvas]:not(:scope > *)');


Do we want to assert that the notes and canvas are inside a slide? That there is only a single instance of each inside each slide?

jamesnw · 2024-01-04T19:21:42Z

slide-deck.js

+      note.toggleAttribute('hidden', this.hideParts === 'note');
+    });
+
+    this.slideCanvas.forEach((canvas) => {


Because this.slideCanvas is assigned before slide setup is called, the contents of this.slideCanvas is only the elements the user applied the attribute to, and not all of the ones that web component applied the attribute to in #setupSlides().

This means that when you hide canvas, you get this-

Only the slides with notes have their canvases hidden.

Yeah, this was intentional, but maybe not the right choice. For slides without separate canvas and notes, hiding the canvas would mean hiding the entire container. That doesn't seem like exactly what we want.

mirisuzanne · 2024-01-04T19:53:56Z

@jamesnw I agree, a view-mode might make sense - and raises some questions about naming.

From an API design perspective, I think we want both:

support for individual manipulation of the primitives (eg you can show either/both in any layout)
a way of adding higher-level concepts that combine them ('presentation mode' shows canvas only, in a full-screen layout)

Ideally, consumers would also be able to add their own higher level modes. I tried to expand on some of the common modes I expect in #12 (comment)

Maybe we need a UX proposal here?

mirisuzanne · 2024-01-05T03:10:11Z

slide-deck.css

  &[blank-slide]::after {
    content: '';
-    background-color: var(--blank-color, black);
+    background-color: var(--blank-color);
    position: absolute;
    inset: 0;
+    z-index: -1;
+  }
+
+  &[blank-slide=black] {
+    --blank-color: var(--color-dark);
+    --text-color: var(--color-light);
  }

  &[blank-slide=white] {
-    --blank-color: white;
+    --blank-color: var(--color-light);
+    --text-color: var(--color-dark);


@stacy I think the purpose of the blank slide is hiding what's on the screen - it's a way to temporarily 'pause' the presentation, either with a white or black overlay. Moving the overlay behind the text, and then adding text contrast kinda defeats the purpose, right?

@mirisuzanne that was not clear to me at all, and for some reason the "blank" in the name made me think it was more like we are starting with a blank slate, a default slide instead of the more obvious intention of "this should not show anything and it should be blank or empty" 🤦🏻‍♀️ . I don't really know what design issues need to be solved in which PRs/Issues right now, and what is a bug vs what isn't yet ready so I feel like I am overthinking things and not being very helpful yet.

That makes sense. I think right now the biggest design issues are the comment above (UX questions about how this should work - what views should be available) and actually designing the slide templates over on the workshop repo. It's ok for design to be pretty minimal in this repo for now.

mirisuzanne · 2024-01-06T01:05:31Z

@stacyk I pushed some updates to the attributes based on our conversation, to try and help make things more clear for styling. It may require a few changes on your end, but I think it gives you more info to work with.

The top level slide element is always labeled as slide-item
If it has a nested canvas, that becomes slide-item="container", otherwise it becomes slide-item="canvas" and also gets the slide-canvas attribute
The active slide always gets aria-current="true"
The slide-deck has a new --slide-count property
Each slide has a new ---slide-index property

Let me know if you need any of those to change.

jamesnw · 2024-01-08T18:29:43Z

I privatized a few more things. Here's the remaining public API, but @mirisuzanne I have a few questions-

DOM Elements - Should these be public? I don't think we want people setting these values, at the very least.
'slides', ~~'slideNotes', 'slideCanvas'~~

Data- these make sense to have public, even when there is overlap with an attribute.
'slideCount','activeSlide', 'slideView', 'keyControl'

Events - is this the right list?
'startEvent', 'resumeEvent', 'joinWithNotesEvent', 'joinEvent', 'endEvent', 'resetEvent', 'blankSlideEvent', 'fullScreenEvent', 'scrollToActive', 'goTo', 'resetActive', 'move', 'goToSaved',

jamesnw · 2024-01-08T18:41:55Z

@mirisuzanne In f5cdd61, I moved the attribute state to getters, which means we can always use the attribute values as the source of truth, and not have to make sure our internal reflection of that value stays up to date. This does mean we need to declare each one instead of adding dynamically, so I'm open to reverting if you'd prefer the old behavior.

slide-deck.js

stacyk · 2024-01-09T05:21:44Z

I didn't get to this one today, but I will look into it tomorrow. Script view definitely isn't ideal for small screens yet.

mirisuzanne · 2024-01-10T17:10:19Z

@stacyk I think when we merge this, it would be a good time to release a new version and update the workshop repo.

stacyk

@mirisuzanne I pushed an update here with some CSS to make script view a bit more readable on smaller screens. And I agree it should go into the workshop repo when approved/released.

I didn't do anything with the slide numbers here, but that can be a different PR.

mirisuzanne

@stacyk looking great - and yeah, I think numbering can wait. I left some style thoughts, but I'm not sure how much we should stress about style, ha.

mirisuzanne · 2024-01-10T22:19:12Z

slide-deck.css

+    grid-template-columns: repeat(auto-fill, minmax(min(50ch, 100%), 1fr));
+  }
+
+  &[slide-view=solo] {
    --slide-height: 100svh;


Maybe this should be set on the grid instead? eg

grid-auto-rows: minmax(100svh, auto);

@mirisuzanne when I played with this, the slides with note don't take up the full height then. Am I missing something?

Oh, yeah, but I think we should be hiding the notes in that view? And then it will work.

@mirisuzanne It looks like we still need the slide height for the container slide-items (or some additional edits to the CSS). The canvas only items are fine with the grid-rows sizing. Just pushed an update that will make it easier to see if you uncheck the --slide-height on [slide-view=solo] [slide-item]

slide-deck.css

mirisuzanne · 2024-01-10T22:30:31Z

slide-deck.css

    --slide-height: 100svh;
  }

+  &[slide-view=script] {
+    --column-gap: calc(1.25em + 2vw);


To me this looks like more space than is needed? I think a faint boarder would be more useful than space? This feels like a view where context is more useful than whitespace.

@mirisuzanne how much space are you seeing? I see like, less than pinky width on active slides?

Official measurement device.

@stacyk Oh oh oh, it's the 5em row gap that's real large for me. Sorry. I think that could be more like a hairline and something closer to the column gap.

I started playing with a hairline (it was a little finicky because of the slide item with container and with only canvas are different sizes), but then I wondered if was more than a layout thing that I should apply in a custom theme instead of the default styles in this tool. But I can play with it more in this tool to see if I can get it just right.

Add slide parts (and mark private methods)

b5f8def

mirisuzanne linked an issue Jan 2, 2024 that may be closed by this pull request

The control-panel should be fully replaceable #9

Closed

mirisuzanne removed a link to an issue Jan 2, 2024

The control-panel should be fully replaceable #9

Closed

This was linked to issues Jan 2, 2024

Feature: Support for speaker notes #4

Closed

Clean up Public & Private API #14

Closed

mirisuzanne requested review from jamesnw, stacyk and jerivas January 3, 2024 04:10

mirisuzanne added 2 commits January 2, 2024 21:31

Clarify sample slide in demo

4bf28c0

Merge branch 'main' into parts

a82b69b

* main: Favicon

mirisuzanne assigned jamesnw, stacyk and jerivas Jan 3, 2024

mirisuzanne mentioned this pull request Jan 3, 2024

Control panel styles and parts #10

Merged

Merge in changes from #10

42a562a

jamesnw requested changes Jan 4, 2024

View reviewed changes

mirisuzanne marked this pull request as ready for review January 4, 2024 19:43

wip - color modes

fc2ed03

mirisuzanne commented Jan 5, 2024

View reviewed changes

stacyk and others added 3 commits January 5, 2024 10:28

revert previous commit

5dea08d

Remove notes show/hide, and add script view

ff5053e

Remove view-specific events

e83e3cb

mirisuzanne unassigned jamesnw and jerivas Jan 5, 2024

Set aria-current for active slides, and clarify part attributes

c92f022

stacyk and others added 3 commits January 5, 2024 23:33

add very basic styles for script view

20b8e36

styles for slide-item in script view

26f76dd

Privatizion

504bdef

Move attribute state to getters

f5cdd61

mirisuzanne commented Jan 8, 2024

View reviewed changes

slide-deck.js Outdated Show resolved Hide resolved

jamesnw added 3 commits January 9, 2024 11:40

Static appendShadowTemplate

fb1b1f6

Make slideNotes and slideCanvas private

c375e08

Add CSS to usage examples, remove version

14de103

jamesnw approved these changes Jan 10, 2024

View reviewed changes

Make script view better for smaller screens

66852dc

stacyk approved these changes Jan 10, 2024

View reviewed changes

mirisuzanne commented Jan 10, 2024

View reviewed changes

stacyk and others added 3 commits January 11, 2024 11:00

order by alpha

15a6c55

adjust solo grid rows for testing, still need slide height though

a4e2519

Stretch slide canvas to fill container in solo view

baa85a5

mirisuzanne merged commit b3c362e into main Jan 11, 2024
4 checks passed

mirisuzanne deleted the parts branch January 11, 2024 17:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add slide parts (and mark private methods) #11

Add slide parts (and mark private methods) #11

mirisuzanne commented Dec 28, 2023 •

edited

Loading

netlify bot commented Dec 28, 2023 •

edited

Loading

mirisuzanne commented Jan 3, 2024

mirisuzanne commented Jan 4, 2024

jamesnw left a comment

jamesnw Jan 4, 2024

jamesnw Jan 4, 2024

jamesnw Jan 4, 2024

jamesnw Jan 4, 2024

mirisuzanne Jan 4, 2024

mirisuzanne commented Jan 4, 2024

mirisuzanne Jan 5, 2024

stacyk Jan 5, 2024

mirisuzanne Jan 5, 2024

mirisuzanne commented Jan 6, 2024

jamesnw commented Jan 8, 2024 •

edited

Loading

jamesnw commented Jan 8, 2024

stacyk commented Jan 9, 2024

mirisuzanne commented Jan 10, 2024

stacyk left a comment

mirisuzanne left a comment

mirisuzanne Jan 10, 2024

stacyk Jan 10, 2024

mirisuzanne Jan 11, 2024

stacyk Jan 11, 2024

mirisuzanne Jan 10, 2024

stacyk Jan 10, 2024

mirisuzanne Jan 11, 2024

stacyk Jan 11, 2024

Add slide parts (and mark private methods) #11

Add slide parts (and mark private methods) #11

Conversation

mirisuzanne commented Dec 28, 2023 • edited Loading

Related Issue(s)

Steps to test/reproduce

netlify bot commented Dec 28, 2023 • edited Loading

✅ Deploy Preview for slide-deck ready!

mirisuzanne commented Jan 3, 2024

mirisuzanne commented Jan 4, 2024

jamesnw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mirisuzanne commented Jan 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mirisuzanne commented Jan 6, 2024

jamesnw commented Jan 8, 2024 • edited Loading

jamesnw commented Jan 8, 2024

stacyk commented Jan 9, 2024

mirisuzanne commented Jan 10, 2024

stacyk left a comment

Choose a reason for hiding this comment

mirisuzanne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mirisuzanne commented Dec 28, 2023 •

edited

Loading

netlify bot commented Dec 28, 2023 •

edited

Loading

jamesnw commented Jan 8, 2024 •

edited

Loading