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

[Enhancement] Always set themeColor default value #2684

Closed
1 of 11 tasks
RasmusKjeldgaard opened this issue Dec 15, 2022 · 0 comments · Fixed by #2735
Closed
1 of 11 tasks

[Enhancement] Always set themeColor default value #2684

RasmusKjeldgaard opened this issue Dec 15, 2022 · 0 comments · Fixed by #2735
Assignees
Labels
enhancement New feature or request NOT Prioritized Issue not yet prioritized and added to a Milestone NOT Tech refined Needs Tech kickoff - solution outlined and agreed

Comments

@RasmusKjeldgaard
Copy link
Collaborator

Describe the enhancement

Right now our themeColor concept is based on an attribute directive that handles theming for a couple of components.

This has one important limitation, as we cannot handle a default theme color on a per component basis. This is because projects have to actually use the themeColor input on components before our custom functionality kicks in and decorates e.g. the avatar with appropriate classes.

This is not a problem for most components, because they just have a default color that themeColor overwrites. But page, card and modal footer can contain other components (like buttons), and at the same time have different default colors to begin with (light for page and modal-footer, white for card). This means the button has no way to know what color its context has.

For example, the button currently assumes that it is on a light background, but when used on a card with its default white background, no context variables are set to inform the button that it is in fact presented on a white background.

This will make the concept outlined in #2602 inconsistent across surfaces in the app.

Describe the solution you'd like

Possibly recreate the themeColor functionality so the themeColor variable is 'owned'/implemented on each component and a default value is set, and initialization logic is run.


Checklist:

The following tasks should be carried out in sequence in order to follow the process of contributing correctly.

Refinement

  • Request that the issue is UX refined; do not proceed until this is done.
  • Request that the issue is tech refined; do not proceed until this is done.

Implementation

The contributor who wants to implement this issue should:

  • Make sure you have read: "Before you get coding".
  • Signal to others you are working on the issue by assigning yourself.
  • Create a branch from the develop branch following our branch naming convention.
  • Publish a WIP implementation to Github as a draft PR and ask for feedback.
  • Make sure you have implemented tests following the guidelines in: "The good: Test".
  • Update the cookbook with examples and showcases.

Review

Once the issue has been implemented and is ready for review:

  • Do a self-review.
  • Create a pull-request. If you created a draft PR during implementation you can just mark that as "ready for review".
@RasmusKjeldgaard RasmusKjeldgaard added enhancement New feature or request NOT Tech refined Needs Tech kickoff - solution outlined and agreed NOT Prioritized Issue not yet prioritized and added to a Milestone labels Dec 15, 2022
@alxzak alxzak added this to Kirby Jan 4, 2023
@alxzak alxzak moved this to 🚀 In Progress in Kirby Jan 4, 2023
@RasmusKjeldgaard RasmusKjeldgaard self-assigned this Jan 5, 2023
@RasmusKjeldgaard RasmusKjeldgaard moved this from 🚀 In Progress to 🔎 Review pending in Kirby Jan 16, 2023
@github-project-automation github-project-automation bot moved this from 🔎 Review pending to ✅ Done in Kirby Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request NOT Prioritized Issue not yet prioritized and added to a Milestone NOT Tech refined Needs Tech kickoff - solution outlined and agreed
Projects
Archived in project
1 participant