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.

Sign up for GitHub

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

Jump to bottom

Added M3 theme to intro pages #756

Merged
merged 5 commits into from May 15, 2024
Merged

Added M3 theme to intro pages #756

merged 5 commits into from May 15, 2024

Conversation

ArnyminerZ
Copy link
Member

@ArnyminerZ ArnyminerZ commented Apr 28, 2024 •
edited by rfc2822

(See #691)

Purpose

Right now our M3 theme is not being used for intro pages.

Short description

  • Added AppTheme call in PageFragment.
  • Added a TODO to remember removing the old M2 theme once all migrations are complete for intro.

Checklist

  • The PR has a proper title, description and label.
  • I have self-reviewed the PR.
  • I have added documentation to complex functions and functions that can be used by other modules.
  • I have added reasonable tests or consciously decided to not add tests.

Depends on #739
Depends on #740
Depends on #741
Depends on #757
Depends on #759

@ArnyminerZ
Added M3 theme
ced1264 
Signed-off-by: Arnau Mora <arnyminerz@proton.me>
@ArnyminerZ ArnyminerZ self-assigned this Apr 28, 2024
@ArnyminerZ ArnyminerZ added the refactoring Internal improvement of existing functions label Apr 28, 2024
@ArnyminerZ ArnyminerZ requested a review from rfc2822 April 28, 2024 08:57
@ArnyminerZ ArnyminerZ marked this pull request as ready for review April 28, 2024 08:58
This was referenced Apr 28, 2024
Merged
Merged
@rfc2822 rfc2822 requested review from sunkup and removed request for rfc2822 April 29, 2024 22:03
rfc2822
rfc2822 requested changes Apr 30, 2024
View reviewed changes
Copy link
Member

@rfc2822 rfc2822 left a comment

Choose a reason for hiding this comment

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

We shouldn't have M2Theme and AppTheme at the same time. We can merge this when all pages are rewritten and then only need AppTheme. I have set appropriate dependencies.

@sunkup
Copy link
Member

sunkup commented Apr 30, 2024

We shouldn't have M2Theme and AppTheme at the same time. We can merge this when all pages are rewritten and then only need AppTheme. I have set appropriate dependencies.

Okay, but why?

@rfc2822
Copy link
Member

rfc2822 commented Apr 30, 2024

I don't want to need multiple PRs for the same file for the rewrite except it's really required without alternative. But I'd prefer that no files are "in-between", but either "old" or "rewritten/finished".

@rfc2822 rfc2822 force-pushed the main-ose branch 15 times, most recently from 74de8dc to 79545a6 Compare May 2, 2024 11:58
@rfc2822 rfc2822 force-pushed the main-ose branch 2 times, most recently from 47ef18b to 39f8f2e Compare May 2, 2024 12:03
@rfc2822 rfc2822 marked this pull request as draft May 10, 2024 09:34
@github-actions GitHub Actions
Copy link

This PR/issue depends on:

@rfc2822 rfc2822 marked this pull request as ready for review May 15, 2024 13:41
@rfc2822 rfc2822 merged commit b533488 into main-ose May 15, 2024
6 checks passed
@rfc2822 rfc2822 deleted the m3-theme-for-intro branch May 15, 2024 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rfc2822 rfc2822 rfc2822 requested changes

@sunkup sunkup sunkup approved these changes

Assignees

@ArnyminerZ ArnyminerZ

Labels
refactoring Internal improvement of existing functions
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ArnyminerZ @sunkup @rfc2822