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 relative move and rotate to rel plugin #794

Merged
merged 10 commits into from Apr 24, 2022

Conversation

thawk
Copy link
Contributor

@thawk thawk commented Aug 22, 2021

The relative position in rel plugin is currently based on the world coordinate. So for the same effect, like fly in from the right-hand side, we must use difference data-rel-x/y/z value. Why not let the plugin do the hard part?

So I introduce a data-rel-position, when set to relative, all relative attribute is based on the position and rotation of previous slide. So no matter the rotation of previous slide, data-rel-x="1000" always looks like fly in from the right-hand side. We can change the position and rotation of one slide, and the position of all following slides will be changed too.

When data-rel-position is set to relative, relative rotation has a clear meaning. It describes the relative rotations between slides. We don't need to set rotations for all slide, setting the key slides is enough. If data-rel-position is not relative, the effect of data-rel-rotate-x/y/z is not clear, so they're only used when data-rel-position="relative".

After the introduction of relative rotation, there're 6 attribute that will inherit from previous slide. If we want to set a relative X move, we have to set all other 5 attributes to 0. It's boring. So a data-rel-clear is used to set all 6 attributes to 0, and then the value specified in current slide is applied. I think this attribute should be used in the second slide of a sequence. The first one setup the basic position, the second set the increments, and the following ones inherit from the second slide.

The examples/3D-positions/indaex.html shows some usage. As you can see, the html code of two slide ring is the same, and slides except for the first two in a ring has no position attributes. It work by inheriting the previous one.

BTW, the test/HOWTO.md says the test js should be placed in the same directory of the plugin itself. But it doesn't point aout where to put the corresponding html file. But in karma.conf.js, only test/plugins/*/*.html is served. So the html files should be put into test/plugins/*? Because I want test js put in the same directory of test html, I have to put the test js into test/plugins/rel/ instead of src/plugins/rel/. Maybe we could update the HOWTO.md to describe where to put html files?

This PR invokes a lot math calculations. Basically, the rotation of a slide is translated into the coordinate describing the directions of X/Y/Z axes. And data-rel-x/y/z can be easily calculated by that. The rotations is the hard part, I mainly use the algorithm in the Quaternions and spatial rotation - Wikipedia to compose two and more rotations.

I'm not a math guy, hope I don't make much mistakes.

Copy link
Contributor

@henrikingo henrikingo left a comment

Choose a reason for hiding this comment

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

Thanks for this PR! You are addressing an area that I left behind when I had just started hacking on impress.js, because I felt it was too complex and I couldn't quite figure out how a user would be likely to use this. I just read this for the first time and made some comments while reading. I'll think about this for a bit and come back with more substantive comments later.

src/plugins/rel/rel.js Outdated Show resolved Hide resolved
// Once rotate-x/rotate-y/rotate-z is set, all three must be set or use default 0
var useRelativeRotate = data.rotateX === undefined &&
data.rotateY === undefined &&
data.rotateZ === undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not convinced this restriction is justified. If a user wants to set rotate-x="90" rotate-rel-y="45" why should we forbid this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rel-rotate-* is based on the status of previous slide, and rotate-* is not, the final rotation may be applied to any axis depends on the rotation of previous slide. I can't image what's the propose and how to mix them.

If we allow the mixing, maybe we should finish the relative rotation, and then apply global rotation? Maybe it will looks like space distortion, just like we place objects in the plain space, and then distort the space. If we first add the angle and then do the rotation, I just can't image what the result is.

Copy link
Contributor

Choose a reason for hiding this comment

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

As an example, and this is how rel-x/y/z work too:

<div class="step" data-rotate-x="45" data-rotate-y="45" >
<div class="step" data-rel-rotate-x="10" data-rotate-y="10" >

The effective absolute values for the second step are now rotate-x=55 and rotate-y=10.

src/plugins/rel/rel.js Outdated Show resolved Hide resolved
src/plugins/rel/rel.js Outdated Show resolved Hide resolved
relRotateZ: el.getAttribute( "data-rel-rotate-z" ),
relPosition: el.getAttribute( "data-rel-position" ),
rotateOrder: el.getAttribute( "data-rotate-order" ),
relRotateOrder: el.getAttribute( "data-rel-rotate-order" )
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand the last one. How could there be a different rotate order for relative vs absolute? I don't think this last attribute is needed, or possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As it's said in examples/3D-rotations/index.html, rotating order matters, relative rotating needs order too. It's ok to use the global data-rotate-order, as we disallow mixing up rotate-x/y/z and rel-rotate-x/y/z, there's no need for two rotation-order in one slide.

Copy link
Contributor

Choose a reason for hiding this comment

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

Rotate order matters, but there can't be two orders effective at the same time.

Also, I'm not going to approve the proposal to disallow mixing relative and absolute coordinates. It's not how rel is designed to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the late reply.
I've modified the plugin to allow mixing relative and absolute coordinates.

step.z = step.z + rel.z;

if ( useRelativeRotate ) {
step.rotate = combineRotations( prev.rotate, step.relative.rotate );
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this not just a sum like the above? What is all the math that follows good for?

Please add your reply as a code comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The rotations in CSS is applied in order, and after each rotation, the rotation coordinate is updated accordingly. So we can't just sum up the angles. We need to know the new coordinate after each rotation, and calculate the position after the rotation according to it, and finally find out a one step rotation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you give some specific examples? I don't follow, and I just don't understand why this needs to be any more complicated than the example I've given in previous comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As it's said in examples/3D-rotations/index.html, order is matter.

For example, in my firefox, the following two slides

<div class="step"  data-rotate-x="90" data-rotate-z="90" data-rotate-order="xyz">XYZ</div>
<div class="step"  data-rotate-x="90" data-rotate-z="90" data-rotate-order="zyx">ZYX</div>

The XYZ one is horizontal and facing upside, the ZYX one is vertical and facing right side.

In my opinion, the XYZ one is first rotated by X-axis, facing upward, and then rotated by the Z-axis of its rotated coordinate, which is the negative Y-axis before rotation. So the result is just like two independent rotation, rotate-x=90 and rotate-y=-90. The ZYX one is similar, the rotated X-axis is the original Y-axis, so it works like rotate-z=90 and then rotate-y=90.

So after each rotation, we have to know what the X/Y/Z-axis is pointing to, and do the following rotations by them. So we can not simply sum up angles of one axis.

For example, rotate-x=90 then rotate-z=90 then rotate-x=90 NOT equals to rotate-x=180 then rotate-z=90.

Hope I explained it clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Furthermore, I suspect sum up the movement while data-rel-position="relative" is also problematic. While data-rel-position="absolute", data-x and data-rel-x are using the same axis, so it can be sum up. But while data-rel-position="relative", data-x and data-rel-x may or maybe use the same axis, so it's not reasonable to be simply summed.

For example, <div class="step" data-x="100" data-rel-y="100">, what's user's purpose?

If previous slide is at (0,0,0), and no rotation, the current slide will be x=100, y=100. If rotate-x=90 in previous slide, the effect will be x=0,y=0, just overlap with the previous one. If the user doesn't know the status of previous slide, how can he know the final status, how can he achieve what he need? If he knows, he should just use the absolute position.

So I think maybe we should disallow mixing up absolute and relative at all, while data-rel-position="relative".

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, order matters and that's why we have the ability to define rotate-order. This is how CSS works. It sounds like you're trying to work against this, and a) I don't quite understand why you're even doing this, and b) it leads to quite complicated code where you're essentially reverse engineering what CSS does in order to counter the rotation from normal CSS.

For example, rotate-x=90 then rotate-z=90 then rotate-x=90 NOT equals to rotate-x=180 then rotate-z=90.

Correct, they are not the same. What I'm trying to say is that rotate-x=180 is the correct choice.

If the user doesn't know the status of previous slide, how can he know the final status, how can he achieve what he need? If he knows, he should just use the absolute position.

Why would the user not know the position of the previous slide? It's the user who created the presentation.

The reason to use rel coordinates is not ignorance, but to save typing.

So I think maybe we should disallow mixing up absolute and relative at all

Again, it really sounds like you are trying to do something different than what is the point of the rel plugin. The rel plugin for sure is meant to mix well on top of the absolute coordinates.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why would the user not know the position of the previous slide? It's the user who created the presentation.

For easy preparing presentation, users may split the presentation into several groups, design each group independently, then finally assemble them into the final presentation. While they design one part, they should only care about the relative position, not the absolute position. The absolute position will be determined at the latest stage, by changed the position of first slide in a group.

For example, in examples/3D-positions/index.html, the presentation is separated into three groups, one box and two rings. Two rings have the same shape, have the same code except for the first slide in each group. When I design them, I really don't know the final position of each slide. And all I need in final is to alter the position of the first slide, to get what I want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It sounds like you're trying to work against this, and a) I don't quite understand why you're even doing this, and b) it leads to quite complicated code where you're essentially reverse engineering what CSS does in order to counter the rotation from normal CSS.

All I want is let the computer do more, the user do less.

When I design a presentation using the originally version of impress.js, I met some problems. I want to separate the presentation in to several parts, in each part, main slides are laid out in a line, slide in from right, and sub slides are laid out vertical below the main slide, slide up one by one. It's easy, right? But while I want to change the direction of slides, trouble comes. E.g. when I want the three parts of slides form a triangle, slides in two of three parts should have an angle of ±60 degree. I found I have to carefully calculate the position of each slide, it is boring. So I thought, why not just tell the computer, I want the next slide at the east side of current slide, and have a distance of 1w? It's easy for computer to do this than we.

So I try finding a way to do this. After some search, I found that quaternion is the most recommended way, so I used it. It's a bit complicate, but I don't know whether there're simplier way.

@henrikingo
Copy link
Contributor

Hi. Sorry for ignoring this great patch, I've been so busy at work this fall.

I took a quick look and am starting to get what you're doing. Also your demo is really nice. Thanks for persisting.

Are you ready from your side? I'll take a deeper look tomorrow/soon.

@thawk
Copy link
Contributor Author

thawk commented Oct 17, 2021

Thx. I just want to make my life more eaier 😆

I don't have more for this function now. But I am not too good at math, some algorithm is developed by myself, I suspect that there may be some error in corner cases.

For further improvement, if we allow specified the rotate center other than the center of slide, we can avoid calculate the center of next slide manually. It's not too hard to implement, but I don't know if we should put it into the rel plugin, or we should rework on the rel plugin, to add some extension point?

@henrikingo
Copy link
Contributor

I just want to make my life more eaier

I know. But what you have done is pretty brilliant now that I see it coming together. I couldn't figure this out 4 years ago when I developed the original rel plugin.

For further improvement, if we allow specified the rotate center other than the center of slide, we can avoid calculate the center of next slide manually. It's not too hard to implement, but I don't know if we should put it into the rel plugin, or we should rework on the rel plugin, to add some extension point?

Ah right. I actually have an opinion on this. Thanks for reminding...

The attribute data-rel-to="" can be used to make the reference point something else than the previous slide. For example all slides can be relative to the first slide of their section. Now, if you want the reference point to not be a slide, you can still insert an empty slide and use the skip CSS class so the slide is skipped when presenting.

**

But now that you bring this up, there's also #733. If we add sections, I could imagine that the <div> for a new section could allow to set a lot of attributes, and setting the coordinate system to another origin could very well be one such thing.

Copy link
Contributor

@henrikingo henrikingo left a comment

Choose a reason for hiding this comment

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

Speaking of data-rel-to ...

prev.relative = {};
prev.relative.position = ref.getAttribute( "data-rel-position" ) || "absolute";
prev.relative.rotate = { x:0, y:0, z:0, order: "xyz" };
Copy link
Contributor

Choose a reason for hiding this comment

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

Wait, why not? IMO both position and rotation should be relative to relTo slide. Basically it should be used 100% as if it's the previous slide.

@thawk thawk force-pushed the rel-position branch 2 times, most recently from 3bf75ad to 474fe47 Compare January 20, 2022 01:21
if ( data.relReset === "all" ) {
inheritRotation = false;
}
} else if ( el.hasAttribute( "data-rel-inherit" ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this not the same as data-rel-to?

I'm trying to remember if I have previously said something that is causing you to not modify that one? In that case I've forgotten, so please either remind me or explain why you would introcude a new attribute that is similar to an existing one?

@thawk
Copy link
Contributor Author

thawk commented Jan 23, 2022

For the newly added data-rel-inherit, this is due to the need for my presentation.
In the presentation, I use 11 slides to make a circle. After some math calculation (using an python script, which uses numpy-quaternion to calculate the relative x/y), we know the relative attributes:

data-rel-rotate-z="32.73" data-rel-x="2059.55" data-rel-y="604.79"

It's quite easy to specify those attributes for the second slide, and the following slides all inherit from it. But in one of those slide, I need to break the sequence by zoom in the slide, then zoom out and resume the sequence to form the circle. How can we resume the sequence? One way is use data-rel-to and double the attributes above again, it works. But if I change my mind to change some relative attributes, I have to change more than one slides. This violates the DRI principle. So I introduce the data-rel-inherit to inherit the relative attributes instead of duplicated them again and again.

The data-rel-inherit can have different target with data-rel-to to increase flexibility. For example, we have two groups of slides: A-B-C-D and E-F-G-H, we can make slide F rel-to E, but rel-inherit B, so the two groups will have the same shape (maybe a square), we can position slide A and E to change the overall position of two groups, and change the attributes of slide B to change the shape of two groups.

This attribute is based on the real need, hope I make it clear.

Maybe I can find some time to make an example.

@henrikingo
Copy link
Contributor

Thanks. I understand the explanation. Let me think about it for a while.

It seems your other PR that was merged, now conflicts with this one for the tests.

@henrikingo
Copy link
Contributor

Ok, so I think what you are trying to do with data-rel-inherit is kind of a form of templating. We could generalize what you are doing and just say that somewhere in the presentation you have a slide:

<div id="slideone" class="step" data-foo="5" data-bar-z="abcd" data-bar-q="99">

Now, later you want to "inherit" some attribute values from this slide to another slide:

<div id="slidefifteen" class="step" data-template-use="slideone" data-foo="6">

This would copy all data-* attributes from slideone, but then set data-foo to 6.

Does this make sense?

@thawk
Copy link
Contributor Author

thawk commented Jan 31, 2022

Thanks. I understand the explanation. Let me think about it for a while.

It seems your other PR that was merged, now conflicts with this one for the tests.

The conflict is raised because both PR add new tests. I will updtae this PR to resolve it.

@thawk
Copy link
Contributor Author

thawk commented Jan 31, 2022

Ok, so I think what you are trying to do with data-rel-inherit is kind of a form of templating. We could generalize what you are doing and just say that somewhere in the presentation you have a slide:

<div id="slideone" class="step" data-foo="5" data-bar-z="abcd" data-bar-q="99">

Now, later you want to "inherit" some attribute values from this slide to another slide:

<div id="slidefifteen" class="step" data-template-use="slideone" data-foo="6">

This would copy all data-* attributes from slideone, but then set data-foo to 6.

Does this make sense?

Yes, it's what I want. But I need to clarify that:

  1. Attributes inherited from slide before the template slide, like data-rel-x/data-rel-rotate-z, should be copied too. Those attributes should be specified in the second slide of a stream, and be inherited one by one. We should follow the previous slide in the same stream, instead of the first/second slide in the stream, so we don't need to know where the stream starts.
  2. I don't quite sure whether other attributes should be copied too. We should copy data-rel-x/data-rel-rotate-x even when they are inherited. But I don't think we should copy data-x if it's inherited. With data-rel-inherit, it's a plugin specified attribute, we can control the behaviour in detail, but as data-template-use, it may need more precise design.

@henrikingo
Copy link
Contributor

henrikingo commented Feb 13, 2022

Sorry for delay. Two weekends of snow blizzard have kept me busy outside.

Attributes inherited from slide before the template slide, like data-rel-x/data-rel-rotate-z, should be copied too. Those attributes should be specified in the second slide of a stream, and be inherited one by one. We should follow the previous slide in the same stream, instead of the first/second slide in the stream, so we don't need to know where the stream starts.

Ok this is a good point. This makes this rather specific to rel plugin. Or even if there was a generic template plugin, it would need access to the internal state of rel plugin. To keep it simple, let's continue within the rel plugin and then if this becomes a popular thing, in the future someone can generalize it.

I don't quite sure whether other attributes should be copied too. We should copy data-rel-x/data-rel-rotate-x even when they are inherited. But I don't think we should copy data-x if it's inherited. With data-rel-inherit, it's a plugin specified attribute, we can control the behaviour in detail, but as data-template-use, it may need more precise design.

So let's probe this a bit more...

In the normal case, both data-x and data-rel-x are inherited from the previous slide, and then of course the effective value of xis essentially the sum of those.

Now if you specify that position should be inherited from some other previous slide, why should that inheritance not work exactly the same?

Copy link
Contributor

@henrikingo henrikingo left a comment

Choose a reason for hiding this comment

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

Reviewing classic-slides changes.

Your work finally solves what I was thinking of when doing this demo. I'm so glad to see this.

Still have questions/thoughts about how we should design the syntax. No more comments on functionality I guess. We're really close now!

</div>
</div>

<div class="step slide" data-rel-x="1600" data-rel-y="1600" data-rotate="60">
<div class="step slide">
Copy link
Contributor

Choose a reason for hiding this comment

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

Beautiful :-)

You've cracked what I wanted to do 5 years ago!

</div>
</div>

<div id="addons" class="step slide title" data-rel-to="motions" data-rel-inherit="motions">
Copy link
Contributor

Choose a reason for hiding this comment

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

Would rel-to and rel-inherit ever be different?

Should we say that inherit implies also rel-to? I think we're missing something in the design here still, but I keep forgetting details we discussed months ago. Sorry.

I'm also thinking you introduced data-rel-position="relative" mode. Would it make sense if the above line would instead just be:

<div id="addons" class="step slide title" data-rel-to="motions" data-rel-position="relative">

...and do exactly the same thing?

(Of course, data-rel-position could also be omitted, since it is itself inherited.)

I think my point here is that you are introducing a new attribute to keep existing syntax backward compatible, but data-rel-position="relative" is already such new syntax where you can define behavior without worrying about backward compatibility.

@thawk
Copy link
Contributor Author

thawk commented Feb 15, 2022

Thanks.
I agree that the current design is a bit more compilicated.
I've introduced the following attributes besides data-rel-rotate-*:

  • data-rel-position: to keep backward compatible
  • data-rel-reset: don't inherit data-rel-x/y/z and data-rel-rotate-x/y/z from previous slide, to avoid keep writing the boring data-rel-x=0 data-rel-y=0...
    • if data-rel-reset="all", use world coordination instead of coordination of the previous slide
  • data-rel-inherit: inherit data-rel-x/y/z and data-rel-rotate-x/y/z from specified slide
    • if no value specified, inherit from the same slide of data-rel-to
    • or inherit from the specified slide

I think the problem is we need TWO slide to begin a sequence: the first slide to position (it may has data-x/y/z and data-rotate-x/y/z), the second slide (it may has data-rel-x/y/z and data-rel-rotate-x/y/z) to determine the relative relationship, and the following slides inherits from the second slide. So, I think the data-rel-to may mostly be used for the first slide, and the data-rel-inherit may be used by the second slide, data-rel-inherit may not the same as data-rel-to.

I think it's better to make a sequence in the SAME slide, then we can simplified the data-rel-inherit and data-rel-to. But if we merge the attributes of the first and the second slide into the first one, it will has data-x/y/z and data-rel-x/y/z at the same time, and use data-x/y/z itself, make data-rel-x/y/z to be inherited by the following slide. I think it may be confusing.

We need to think about that we need, and make a reasonable syntax.

@henrikingo
Copy link
Contributor

Thanks for the summary!

Backward compatibility adds some complexity, but it's a necessary evil. IMO the fact that absolute/relative map clearly to the same options in CSS positioning makes this a nice interface to the use to digest.

So I think everything else makes sense, and we should just continue to understand what to do with data-rel-to and data-rel-inherit. I think the way you explain it it sounds logical. However, did you actually create any presentation where you needed to separate the two?

Also, you should ask yourself whether you are over optimizing? Using data-rel-to and especially data-rel-inherit is already the exceptional case. Maybe it's better to have the user repeat a few attributes if needed, rather than introducing one more command that is only slightly different from another command.

@henrikingo
Copy link
Contributor

Just to keep the conversation going: What do you think about the KISS proposal: Just remote data-rel-inherit and merge its functionality with data-rel-to?

@thawk
Copy link
Contributor Author

thawk commented Mar 20, 2022

Sorry for the late reply. I'm a bit busy and not have much time recentlly due to the COVID-19.

I agree that we can remove data-rel-inherit, and let data-rel-to inherits data-rel-rotate-* also, it can be usefull when we temporary leave the flow and then resume the flow. We can just data-rel-to the previous slide in the flow, works like the out-of-flow slides are not exists. We can use data-rel-reset to get the old behavior.

When we remove data-rel-inherit, we CAN'T inherit data-rel-* from the specified slide without inherit it's position. But I think maybe it isn't a strong need, and can be achieved by copy the attributes again.

@thawk
Copy link
Contributor Author

thawk commented Mar 20, 2022

The problem confuses me is what we should deal with data-rel-rotate-* when data-rel-position no equals relative, should those attributes be inherited?
Not the unittest failed when the data-rel-position is not set, the current implement DON'T inherit data-rel-rotate-* as before data-rel-position was introduced, it make the test result not reasonable. It's behavior is clear when data-rel-position="relative", so it succeed then.

@impress impress deleted a comment from fzx18046715594 Mar 23, 2022
@impress impress deleted a comment from noahlias Mar 23, 2022
@henrikingo
Copy link
Contributor

Sorry for the late reply. I'm a bit busy and not have much time recentlly due to the COVID-19.

Sorry to hear this. Don't apologize. This is a hobby project and my responses aren't always prompt either.

When we remove data-rel-inherit, we CAN'T inherit data-rel-* from the specified slide without inherit it's position. But I think maybe it isn't a strong need, and can be achieved by copy the attributes again.

Since you've highlighted break from flow as a major use case, it seems you'd want to specify those anyway.

The problem confuses me is what we should deal with data-rel-rotate-* when data-rel-position no equals relative, should those attributes be inherited?

No, I don't see how that could make sense. NOT inheriting seems correct.

@thawk
Copy link
Contributor Author

thawk commented Mar 26, 2022

Considering the original behaviours, I make the following changes when data-rel-position isn't relative:

  • once a slide is set with data-rel-rotate-*, the following slides will keep the same rotations

    • data-rel-rotate-* should be inherited
      • It says that "Non-zero values are also inherited", so the data-rel-rotate-* shouldn't be an exception
    • Because the data-rotate-* will not be inherited, so if data-rotate-* is not set, data-rel-rotate-* will be the final data-rotate-*

    E.g., when a slide has data-rel-rotate-z="90", the slide itself and following slides all act as if data-rotate-z="90" is set for them. It helps avoid repeating data-rotate-z="90" in multiple slides.

  • data-rel-to don't inherit data-rel-*. I think it's the orignal behaviour.

The unittest is passed now.

@thawk
Copy link
Contributor Author

thawk commented Mar 26, 2022

For the slide flow, for example, I have a series of slides, they all inherits the same relative attributes:

Slide1 -> Slide2 -> Slide3 -> Slide4

Some slide may need a detail explain, then return the the original slide. The detail slide is out-of-flow.

Slide1 -> Slide2 -> Slide3    Slide4
                       v        ^
                   Slide3.1 ----+

It may be hard to calculate the position of Slide4 based on Slide3.1, but it's easy to do based on Slide3, so it's better to inherit from Slide3, like the slide flow is not been interrupted.

@henrikingo
Copy link
Contributor

data-rel-to don't inherit data-rel-*. I think it's the orignal behaviour.

Otherwise all following slides would end up in the same position. (Same delta relative to data-rel-to slide.)

@henrikingo
Copy link
Contributor

So is this ready to merge now?

@thawk
Copy link
Contributor Author

thawk commented Apr 7, 2022

So is this ready to merge now?

Maybe you can check whether the old presentations works as expected, if so, I think it's ready.

@henrikingo
Copy link
Contributor

Good idea. They all look ok.

@henrikingo
Copy link
Contributor

Is your first comment still usable as a commit message? Or do you want to provide a commit message for the squashed commit?

@thawk
Copy link
Contributor Author

thawk commented Apr 15, 2022

Is your first comment still usable as a commit message? Or do you want to provide a commit message for the squashed commit?

English is not my native language. You decide. I don't mind if you rewrite a new comment.

@henrikingo
Copy link
Contributor

Nor is it mine :-) I was more checking whether it's still accurate after changes. But I'll use it. Thanks.

@henrikingo henrikingo merged commit 629f768 into impress:master Apr 24, 2022
@henrikingo
Copy link
Contributor

And there it is!

Hey, I just wanted to come back and say THANK YOU. This is probably the coolest, most advanced patch I've seen since I started re-activating impress.js project. If you are interested to work on more stuff, please keep in touch and let me know if there's anything I can do to help?

henrik

@thawk thawk deleted the rel-position branch April 25, 2022 07:56
@thawk
Copy link
Contributor Author

thawk commented Apr 25, 2022

Yes, I will keep using impress.js, it's a great presentation tool. I will fire some issues or PR if necessary.

@impress impress deleted a comment from fzx18046715594 Oct 11, 2022
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