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

Page-Duration #46

Open
erkilic opened this issue Oct 26, 2020 · 14 comments
Open

Page-Duration #46

erkilic opened this issue Oct 26, 2020 · 14 comments

Comments

@erkilic
Copy link

erkilic commented Oct 26, 2020

As an idea - for some users it might be helpful to adjust individuall duration time for each page.
This would have the advantage, that pages with more data can rest longer, where other pages with less information can be slided faster.
Furthermore the user would have the possibility to give a page the duration "0" seconds, if he doesn't wants to show the module although it should be loaded and operationg in the background (e.g. MPlayerRadio).
Would this new feature be possible? I would appreciate it very :-)

@edward-shen
Copy link
Owner

edward-shen commented Nov 1, 2020

Sorry for the late reply, things have gotten a little hectic.

it might be helpful to adjust individuall duration time for each page.

This seems like a good idea, but I'm a little busy right now to implement it. I'd happily accept a PR for this, but I'm likely unable to fufil requests until the end of the year.

Furthermore the user would have the possibility to give a page the duration "0" seconds, if he doesn't wants to show the module although it should be loaded and operationg in the background (e.g. MPlayerRadio).

I believe this can be achieved either through not setting a position in the config for that module, or a combination where you set the module as a fixed module and hiding it through CSS.

@erkilic
Copy link
Author

erkilic commented Nov 1, 2020 via email

@DOA1991
Copy link

DOA1991 commented Apr 9, 2021

Really would like to see an option for individual page duration time as well. is there any chance that this will be implemented?

Thanks :)

@soubhik-khan
Copy link

+1 to individual page duration

@edward-shen
Copy link
Owner

I'm currently looking for a new maintainer for my magic mirror modules, so I won't be implementing any new features myself. PRs are always appreciated, but don't expect feature requests to be implemented unless a new maintainer is found.

@sdetweil
Copy link

sdetweil commented Nov 21, 2024

@KristjanESPERANTO
Copy link
Collaborator

I also created a PR (#88). In my opinion, my approach (with a simple array) has two advantages: no major code changes were necessary and the configuration is somewhat clearer and therefore simpler.

Here for comparison what the configuration of my PR and the other PR look like (if I have understood the other PR correctly):

My PR #88:

            individualRotationTimes: [
            	5000,	// 5 seconds for page 1
            	10000,	// 10 seconds for page 2
            	5000,	// 5 seconds for page 3
            	15000,	// 15 seconds for page 4
            ],

PR #76:

            pageTimeout: [
            	{ pageNumber: 1, timeout: 5000 },	// 5 seconds for page 1
            	{ pageNumber: 2, timeout: 10000 },	// 10 seconds for page 2
            	{ pageNumber: 3, timeout: 5000 },	// 5 seconds for page 3
            	{ pageNumber: 4, timeout: 15000 },	// 15 seconds for page 4
            ],

@sdetweil
Copy link

sdetweil commented Feb 2, 2025

The point of my page Number field was to not require the exact symmetry between extra info and specific page timing

Have 8 pages, want page 6 longer, takes 1 entry, not 8, and if you wanted to change another, add one entry, order not affecting the results.

@KristjanESPERANTO
Copy link
Collaborator

I can see the advantages of your approach. But I think the simplicity of my approach, both in the config and in the code, might outweigh it.

To illustrate, here are the configs of both approaches, where only the 8th page is different:

PR 76

            pageTimeout: [
            	{ pageNumber: 8, timeout: 60000 },
            ],

PR 88

Long version:

            individualRotationTimes: [
            	15000,	// page 1
            	15000,	// page 2
            	15000,	// page 3
            	15000,	// page 4
            	15000,	// page 5
            	15000,	// page 6
            	15000,	// page 7
            	60000,	// page 8
            ],

Short version:

            individualRotationTimes: [ 15000, 15000,  15000, 15000, 15000, 15000, 15000, 60000 ],

Without causing a breaking change or significant increase in code complexity, I can now only see these two practicable approaches. Even though I like the simplicity of my approach, I would also be okay with sdetweil's. @edward-shen What do you think?

@sdetweil
Copy link

sdetweil commented Feb 2, 2025

You could also use 0 for the pages that use the default timing mechanism

This new property is optional

I did the same design implementation for MMM-Carousel, and MMM-ModulesGroupsRotation
(carousel first)(trying to be consistent at the user experience)

@KristjanESPERANTO
Copy link
Collaborator

KristjanESPERANTO commented Feb 2, 2025

You could also use 0 for the pages that use the default timing mechanism

Sure, but I can't see much advantage in that. I rather see that docs and code would become more complex and error-prone.

I did the same design implementation for MMM-Carousel ... (trying to be consistent at the user experience)

I also contribute to the "official" fork of MMM-Carousel. I can't see this function there yet. If you have only done this in your fork, I consider the consistent argument to be invalid.

@edward-shen
Copy link
Owner

For bigger number of pages, I do like Sam's implementation, but I like how terse a list is. I am a fan of explicit over implicit. How about a mix?

let timings = {
  "default": 3000,
  "1": 500,
  "4": 600,
};

timings[1] || timings["default"] // 500
timings[40] || timings["default"] // 3000

At the very least I would like to see an implementation where a default value is supported if the timing isn't found. Currently, neither implementation supports falling back.

Also bonus points if you add support for hidden/named pages.

@sdetweil
Copy link

sdetweil commented Feb 3, 2025

Mine only uses the extra if specified, default otherwise. User only adds difference from default config

I thought named/hidden were by notification only. And not timed

@sdetweil
Copy link

sdetweil commented Feb 3, 2025

@KristjanESPERANTO Carousel was not updated for a long while too. so I made changes to my fork. I don't keep track of who is doing what to what.

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

No branches or pull requests

6 participants