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

fix(theme): fixed that all disabled elements used a disable background color instead of just disabled tabs #122

Conversation

christoph-jerolimov
Copy link
Member

Hey, I just made a Pull Request!

TODO

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

@rhdh-gh-app
Copy link

rhdh-gh-app bot commented Dec 4, 2024

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/backstage-plugin-mui4-test workspaces/theme/plugins/mui4-test none v0.1.0
@red-hat-developer-hub/backstage-plugin-mui5-test workspaces/theme/plugins/mui5-test none v0.1.0
@red-hat-developer-hub/backstage-plugin-theme workspaces/theme/plugins/theme patch v0.4.0

Copy link
Contributor

@ciiay ciiay left a comment

Choose a reason for hiding this comment

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

Disabled buttons are fixed, just left a comment about the tab button being gray issue.

@@ -436,7 +436,7 @@ export const createComponents = (themeConfig: ThemeConfig): Components => {
root: {
textTransform: 'none',
minWidth: 'initial !important',
'&.Mui-disabled': {
'&:disabled': {
backgroundColor: general.disabledBackground,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
backgroundColor: general.disabledBackground,
backgroundColor: 'unset',

⬆️ fixes the following issue with tab button background color being gray.
image

Copy link
Member Author

Choose a reason for hiding this comment

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

Then I can just remove this disabled code. I thought setting a gray background to the tab was exactly what we want to make it more clear that the tab is disabled.

With just the different text Color is really hard to see if a tab is disabled. I will check with @ShiranHi and the PF 5/6 documentation.

Copy link
Member Author

@christoph-jerolimov christoph-jerolimov Dec 5, 2024

Choose a reason for hiding this comment

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

@ciiay yeah this "idea" comes from PF. See: https://v5-archive.patternfly.org/components/tabs

image

image

Copy link

Choose a reason for hiding this comment

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

Right, let's follow PF guidelines. Do they use the same gray color for both light and dark mode?

Copy link
Member Author

Choose a reason for hiding this comment

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

@ShiranHi, I like that you asking the uncomfortable, on-point questions 🤣

Here we go: #129

…d color instead of just disabled tabs

Signed-off-by: Christoph Jerolimov <[email protected]>
@christoph-jerolimov christoph-jerolimov force-pushed the fix-disabled-background-color branch from 5faeb4b to 249c5fa Compare December 5, 2024 01:25
Copy link
Contributor

@ciiay ciiay left a comment

Choose a reason for hiding this comment

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

/lgtm

Thank you 🤝

@openshift-ci openshift-ci bot added the lgtm label Dec 5, 2024
@ciiay ciiay merged commit 037324e into redhat-developer:main Dec 5, 2024
7 checks passed
@christoph-jerolimov christoph-jerolimov deleted the fix-disabled-background-color branch December 5, 2024 01:32
christoph-jerolimov added a commit to christoph-jerolimov/rhdh-plugins that referenced this pull request Dec 5, 2024
@christoph-jerolimov christoph-jerolimov self-assigned this Dec 5, 2024
christoph-jerolimov added a commit that referenced this pull request Dec 6, 2024
…elevations (#126)

* fix(theme): fix/remove different paper background colors/shades in MUI v5

Signed-off-by: Christoph Jerolimov <[email protected]>

* fix(theme): remove unused Mui-Disabled workaround after #122 was merged

Signed-off-by: Christoph Jerolimov <[email protected]>

* fix(theme): replace the paper shadow and added a border to all paper elevations

Signed-off-by: Christoph Jerolimov <[email protected]>

---------

Signed-off-by: Christoph Jerolimov <[email protected]>
04kash pushed a commit to 04kash/rhdh-plugins that referenced this pull request Dec 12, 2024
…d color instead of just disabled tabs (redhat-developer#122)

Signed-off-by: Christoph Jerolimov <[email protected]>
04kash pushed a commit to 04kash/rhdh-plugins that referenced this pull request Dec 12, 2024
…elevations (redhat-developer#126)

* fix(theme): fix/remove different paper background colors/shades in MUI v5

Signed-off-by: Christoph Jerolimov <[email protected]>

* fix(theme): remove unused Mui-Disabled workaround after redhat-developer#122 was merged

Signed-off-by: Christoph Jerolimov <[email protected]>

* fix(theme): replace the paper shadow and added a border to all paper elevations

Signed-off-by: Christoph Jerolimov <[email protected]>

---------

Signed-off-by: Christoph Jerolimov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants