Skip to content

Support @include directives in QSS templates - #14101

Open
kaamyashinde wants to merge 2 commits into
implement-basis-for-new-stylingfrom
qss-include-directive
Open

Support @include directives in QSS templates#14101
kaamyashinde wants to merge 2 commits into
implement-basis-for-new-stylingfrom
qss-include-directive

Conversation

@kaamyashinde

@kaamyashinde kaamyashinde commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Issue
Resolves #14103

Approach
Stacked on #14100 so that needs to be reviewed first.

base.qss.in holds styling for the whole GUI, which won't scale. Adds @include "nav.qss.in" so stylesheets can be split per GUI area. Includes are resolved before token substitution, so included files use {{token}} like the base template.

Includes may nest; circular references raise QssProcessingError instead of recursing until the stack is exhausted.
No visual change, base.qss.in uses no includes yet, so the rendered stylesheet is unchanged.

  • PR title captures the intent of the changes, and is fitting for release notes.
  • Added appropriate release note label
  • Commit history is consistent and clean, in line with the contribution guidelines.
  • Make sure unit tests pass locally after every commit (git rebase -i main --exec 'just rapid-tests')

When applicable

  • When screenshots are changed: Review screenshot-PR in ert-testdata,
    merge screenshot-PR in ert-testdata before merging this PR.
  • When there are user facing changes: Updated documentation
  • New behavior or changes to existing untested code: Ensured that unit tests are added (See Ground Rules).
  • Large PR: Prepare changes in small commits for more convenient review
  • Bug fix: Add regression test for the bug
  • Bug fix: Add backport label to latest release (format: 'backport release-branch-name')

@kaamyashinde
kaamyashinde marked this pull request as draft August 5, 2026 11:03
@codecov-commenter

codecov-commenter commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.93%. Comparing base (b359739) to head (da0b629).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@                       Coverage Diff                        @@
##           implement-basis-for-new-styling   #14101   +/-   ##
================================================================
  Coverage                            91.93%   91.93%           
================================================================
  Files                                  483      483           
  Lines                                33505    33525   +20     
================================================================
+ Hits                                 30802    30822   +20     
  Misses                                2703     2703           
Flag Coverage Δ
cli-tests 36.19% <0.00%> (+0.02%) ⬆️
fuzz 44.22% <19.04%> (-0.02%) ⬇️
gui-tests 58.55% <0.00%> (-0.04%) ⬇️
performance-and-unit-tests 80.75% <100.00%> (+0.01%) ⬆️
test 46.01% <0.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/ert/gui/theme_manager/qss_processing.py 100.00% <100.00%> (ø)

@kaamyashinde
kaamyashinde force-pushed the qss-include-directive branch from 369c272 to 0cbf763 Compare August 5, 2026 11:17
@kaamyashinde
kaamyashinde force-pushed the qss-include-directive branch from 0cbf763 to c3b669d Compare August 5, 2026 11:45
@codspeed-hq

codspeed-hq Bot commented Aug 5, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 36 untouched benchmarks


Comparing qss-include-directive (da0b629) with implement-basis-for-new-styling (1b33067)1

Open in CodSpeed

Footnotes

  1. No successful run was found on implement-basis-for-new-styling (b359739) during the generation of this report, so 3d6e88c was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds support for @include "…" directives in QSS template processing so GUI styling can be split across multiple .qss.in files while still using design-token substitution.

Changes:

  • Introduce resolve_includes() to expand @include "file.qss.in" directives (including nested includes) and detect circular references.
  • Update process_qss() to resolve includes before token substitution.
  • Move/extend unit tests to cover include resolution behavior and keep resource-reading tests centralized.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/ert/gui/theme_manager/qss_processing.py Adds include-resolution pre-pass and integrates it into QSS processing.
tests/ert/unit_tests/gui/theme_manager/test_qss_processing.py Adds unit tests for include resolution (multiple, nested, circular, missing).
tests/ert/unit_tests/gui/theme_manager/test_theme_utils.py Relocates and keeps tests for read_qss_stylesheet_file() behavior.

Comment on lines 8 to 11
_TOKEN_PATTERN = re.compile(r"\{\{([a-z0-9\-]+)\}\}")
_INCLUDE_PATTERN = re.compile(r'^@include\s+"(.+?)"', re.MULTILINE)

_BASE_TEMPLATE = "base"
Comment thread tests/ert/unit_tests/gui/theme_manager/test_qss_processing.py
@kaamyashinde
kaamyashinde force-pushed the qss-include-directive branch from 8abd255 to 75e7ef8 Compare August 6, 2026 07:09
@kaamyashinde
kaamyashinde force-pushed the qss-include-directive branch from 75e7ef8 to 0b4c9f6 Compare August 6, 2026 07:42
@kaamyashinde
kaamyashinde force-pushed the qss-include-directive branch from 0b4c9f6 to 32a599e Compare August 6, 2026 08:34
@ertomatic

Copy link
Copy Markdown
Collaborator

Screenshots differ from baselines. A baseline update PR has been prepared: equinor/ert-testdata#80

Allow a .qss.in template to pull in another with @include "nav.qss.in",
so styling can be split per GUI area instead of living in one file.
Includes are resolved before token substitution, so included files use
{{token}} placeholders exactly like the base template.

Includes may nest; circular references raise QssProcessingError instead
of recursing until the stack is exhausted.
read_qss_stylesheet_file only names a resource and delegates to
read_theming_resource, so its tests belong with the other resource
loader tests rather than among the template processing ones.
@kaamyashinde
kaamyashinde force-pushed the qss-include-directive branch from 32a599e to da0b629 Compare August 6, 2026 11:11
@ertomatic

Copy link
Copy Markdown
Collaborator

Screenshot tests now pass. The baseline update PR equinor/ert-testdata#80 has been closed.

@kaamyashinde
kaamyashinde requested review from SAKavli and eilskra August 6, 2026 11:43
@kaamyashinde
kaamyashinde marked this pull request as ready for review August 6, 2026 11:44
Comment on lines +30 to +36
for match in _INCLUDE_LIKE_PATTERN.finditer(template):
line = match.group(0)
if not _INCLUDE_PATTERN.fullmatch(line):
raise QssProcessingError(
f"Malformed @include directive: {line.strip()}. "
'Expected the form: @include "name.qss.in"'
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need to separate the two pattern checks? What happens if we just use _INCLUDE_PATTERN from the start?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What are the pros/cons of having multiple .qss.in? Do we have enough components to that we need the added complexity of having between-qss communication?

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.

Allow for using a base.qss.in as a single point of entry

5 participants