Skip to content
This repository was archived by the owner on Apr 2, 2024. It is now read-only.

Conversation

@tokewf
Copy link
Contributor

@tokewf tokewf commented Oct 19, 2021

What it says on the tin. Removes custom_theme dependency so that the module can be used in projects that don't have that theme. Sets default theme to Gin instead.

@tokewf tokewf requested a review from madsnorgaard October 19, 2021 06:39
@tokewf tokewf self-assigned this Oct 19, 2021
@madsnorgaard madsnorgaard requested a review from lats1 October 19, 2021 08:29
@madsnorgaard
Copy link
Collaborator

@lats1 Dette er en del af Magentas tilrettelser til brugergrænsefladen - specifikt ved at ebnytte Gin fremfor custom_theme.

Dettte skal sikre kontinuitet og at vi kan tilrette felter/formualrer til også at overholde de fælles design principper uden at benytte custom_theme.

Copy link
Collaborator

@madsnorgaard madsnorgaard left a comment

Choose a reason for hiding this comment

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

LGTM

@lats1
Copy link
Contributor

lats1 commented Oct 19, 2021

Isn't this more suited for the profile? @madsnorgaard And haven't we talked about moving gin as dependency to the profile instead of the module?

@madsnorgaard madsnorgaard self-requested a review October 19, 2021 08:40
@tokewf
Copy link
Contributor Author

tokewf commented Oct 19, 2021

Isn't this more suited for the profile? @madsnorgaard And haven't we talked about moving gin as dependency to the profile instead of the module?

I am currently testing this. May I suggest, then, that this module sets neither custom_theme nor gin as the default theme, but simply leave that responsibility to whichever profile is used?

@madsnorgaard
Copy link
Collaborator

Absolutely!

@tokewf
Copy link
Contributor Author

tokewf commented Oct 20, 2021

@madsnorgaard @lats1 I (think I) removed everything related to theme configuration from this module. Can you please re-review and verify that I didn't remove too much?

… shouldn't make decisions about which theme to use. Leave that up to whichever install profile is being used.
@tokewf tokewf force-pushed the feature/46203_remove_custom_theme branch from 6434470 to 6fbfd75 Compare October 20, 2021 08:30
@tokewf tokewf merged commit 2fd720a into develop Oct 20, 2021
@tokewf tokewf deleted the feature/46203_remove_custom_theme branch October 20, 2021 08:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants