Skip to content

Conversation

mrblomblo
Copy link
Contributor

@mrblomblo mrblomblo commented Sep 13, 2025

Added dynamic loading of theme previews, and switched the preview type to iframe. Other than that, I made some small aesthetic improvements such as: emphasis color on the border of the preview of the currently applied theme, more vertically centered theme preview title, and a small space to the left of the radio.

image

In the picture, you may notice that a theme or two doesn't have the correct colors on the generate button. I believe that is caused by the theme not explicitly defining said color in its theme file. I think I may know a fix for this issue.
EDIT: The above guess was not close to the real issue at all.

- Remove notice in install hint
- Remove lazy-loading of iframes
- Load CSS in iframe with C# instead of JS
@mrblomblo
Copy link
Contributor Author

mrblomblo commented Sep 13, 2025

I fixed the things you brought up, but I still don't get why the iframes are inheriting the variable values from the parent page. It even seems to have gotten worse now after setting the theme with C# for some reason. I've already tried the following to fix it, but none of my attempts have even been slightly successful:

  • Add contain: layout style paint; and isolation: isolate; to the CSS for the iframe element in the parent page
  • Add sandbox="allow-same-origin allow-scripts" to the iframe element in the parent page
  • Change all variable values from their current values to both unset and initial before loading the theme both inside the iframe and the iframe element in the parent page, both with and without !important (i.e., try to reset them)
  • Set all: unset; for :root in the iframe
  • Change the CSS file-loading order in the iframe

@mrblomblo
Copy link
Contributor Author

I suppose I could test web components. If that doesn't work, then I will probably close this PR and create a new one with just the dynamic theme preview loading (for a clean commit history) and leave the improved preview part to someone more capable than me.

@bman654
Copy link
Contributor

bman654 commented Sep 17, 2025

I fixed the things you brought up, but I still don't get why the iframes are inheriting the variable values from the parent page. It even seems to have gotten worse now after setting the theme with C# for some reason. I've already tried the following to fix it, but none of my attempts have even been slightly successful:

  • Add contain: layout style paint; and isolation: isolate; to the CSS for the iframe element in the parent page
  • Add sandbox="allow-same-origin allow-scripts" to the iframe element in the parent page
  • Change all variable values from their current values to both unset and initial before loading the theme both inside the iframe and the iframe element in the parent page, both with and without !important (i.e., try to reset them)
  • Set all: unset; for :root in the iframe
  • Change the CSS file-loading order in the iframe

what specifically is happening?

@mrblomblo
Copy link
Contributor Author

I fixed the things you brought up, but I still don't get why the iframes are inheriting the variable values from the parent page. It even seems to have gotten worse now after setting the theme with C# for some reason. I've already tried the following to fix it, but none of my attempts have even been slightly successful:

  • Add contain: layout style paint; and isolation: isolate; to the CSS for the iframe element in the parent page
  • Add sandbox="allow-same-origin allow-scripts" to the iframe element in the parent page
  • Change all variable values from their current values to both unset and initial before loading the theme both inside the iframe and the iframe element in the parent page, both with and without !important (i.e., try to reset them)
  • Set all: unset; for :root in the iframe
  • Change the CSS file-loading order in the iframe

what specifically is happening?

Ah, sorry for not saying what's happening... Here's a screenshot of how it looks in its current state:

image

Basically, it seems like all theme preview iframes inherit the CSS variable values from the parent page (the install page) at load (they don't change styling when changing the theme preview), ignoring the variable values they are given by the theme file they're supposed to be showing off in the actual theme preview iframe. Well, apart from Punked, which seems to have a higher specificity for the generate button and middle generate button in its CSS file.

I hope I made sense lol

@mcmonkey4eva
Copy link
Member

more likely you're just seeing the default cookie-based theme application and not replacing it properly?

@mrblomblo
Copy link
Contributor Author

That could definitely be the issue, I hadn't thought of that

@mrblomblo
Copy link
Contributor Author

mrblomblo commented Sep 18, 2025

Also, do you think it'd be better to have a (Legacy) badge after legacy themes' names, similar to how it looks in the theme picker in the user settings, or leave it the way it is now? @mcmonkey4eva

@mcmonkey4eva
Copy link
Member

leave that as-is

@mrblomblo
Copy link
Contributor Author

mrblomblo commented Sep 20, 2025

Theme previews now work properly.
image

@mrblomblo
Copy link
Contributor Author

I've implemented the ViewData thing you suggested. I am trying to find the Core Parameters so I can copy it over, but I can't find it

@mcmonkey4eva
Copy link
Member

Parameters on the generate tab are generated by JS. makeSliderInput and all those

@mrblomblo
Copy link
Contributor Author

I am getting nowhere. I cannot figure out how to get the params to be generated by JS, and I cannot find any code I can use as an example in any of the files.

@mcmonkey4eva
Copy link
Member

Search the function I named above in vs code?

image

@mrblomblo
Copy link
Contributor Author

Sorry, was having a slow day. I am making progress right now, but the Seed input buttons are acting a little weird:
Modern theme:
image

Legacy theme:
image

Current code
function reuseLastParamVal() {
    return null; // Prevent error in browser console
}

function generateInputs() {
    const container = getRequiredElementById('input_group_content_coreparameters');
    container.innerHTML = 
        makeNumberInput(null, 'input_images', 'images', 'Images', '', 1, 1, 10000, 1, 'big', false, true)
        + makeNumberInput(null, 'input_seed', 'seed', 'Seed', '', -1, -1, 9223372036854776000, 1, 'seed', false, true)
        + makeSliderInput(null, 'input_steps', 'steps', 'Steps', '', 20, 0, 500, 0, 100, 1, false, false, true)
        + makeSliderInput(null, 'input_cfgscale', 'cfgscale', 'CFG Scale', '', 7, 0, 100, 0, 20, 0.5, false, false, true);
    autoNumberWidth(document.getElementById('input_images'));
    enableSliderAbove(document.getElementById('input_steps'));
    enableSliderAbove(document.getElementById('input_cfgscale'));
}

document.addEventListener('DOMContentLoaded', function() {
    generateInputs();
});

Also, should I make the popover buttons functional, or is that a bit overkill?

@mcmonkey4eva
Copy link
Member

None of the buttons should do anything when clicked (other than basic buttonstate stuff)

Known issue: two icons are present in the seed buttons in modern themes
@mrblomblo
Copy link
Contributor Author

Core params are here now, but two icons are present in the seed buttons in modern themes...
image

@mcmonkey4eva
Copy link
Member

but two icons are present in the seed buttons in modern themes...
That was caused by some jank css in modern.css - fixed now, and I merged master to this PR, it does indeed fix the issue here.

This is looking really solid. Just needs some minor refinements I think and it's ready to go.

- Add missing semicolon in themepreview CSS
- Remove unnecessary popover creation logic
- Fix label CSS in installer CSS
@mrblomblo
Copy link
Contributor Author

I fixed the things you brought up.

What would you think of having the background of the previews use the same noise as the generate tab for extra immersion?
imageimage

@mrblomblo mrblomblo marked this pull request as ready for review October 4, 2025 11:12
@mcmonkey4eva
Copy link
Member

yeah the noise background texture should be included where relevant yeh

@mrblomblo
Copy link
Contributor Author

Noise image is now present in the core params section.
image

Btw, when I was doing this, I noticed that the noise image isn't applied to light themes. Is that intentional?

@mrblomblo
Copy link
Contributor Author

mrblomblo commented Oct 4, 2025

Oh yeah, in the browser console there is an error about recursion that keeps spamming the browser error log whenever you click in a preview. It stops spamming when you click outside the preview.

Screencast_20251004_232305.mp4

(Sorry about the laggy recording)

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.

3 participants