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) Fix ContentSwitcher style overrides #773

Merged
merged 2 commits into from
Sep 26, 2023

Conversation

denniskigen
Copy link
Member

@denniskigen denniskigen commented Sep 26, 2023

Requirements

  • This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.

For changes to apps

If applicable

  • My work includes tests or is validated by existing tests.
  • I have updated the esm-framework mock to reflect any API changes I have made.

Summary

In this PR, I've addressed some styling inconsistencies with the Carbon ContentSwitcher component following the recent Carbon version bump.

This PR does not fix an issue where the size prop of the ContentSwitcher component is getting ignored:

The value of the size prop of the ContentSwitcher is set to sm but it gets ignored

CleanShot 2023-09-26 at 4 07 53@2x

The size prop of the ContentSwitcher is set to sm but is getting overriden by the var(--cds-layout-size-height-local) value of the height property.

styling affecting height

This is owing to an upstream change where the ContentSwitcher got wrapped in a LayoutConstraint component. I'll address this using either an override or a tweak to the relevant layout token.

Layout constraints explainer.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 26, 2023

Size Change: -323 kB (-10%) 👏

Total Size: 2.81 MB

Filename Size Change
packages/shell/esm-app-shell/dist/openmrs.a7afcf84e4ced6ef.js 0 B -323 kB (removed) 🏆
ℹ️ View Unchanged
Filename Size
packages/apps/esm-devtools-app/dist/68.js 4.29 kB
packages/apps/esm-devtools-app/dist/735.js 2.63 kB
packages/apps/esm-devtools-app/dist/889.js 175 kB
packages/apps/esm-devtools-app/dist/988.js 282 B
packages/apps/esm-devtools-app/dist/main.js 2.95 kB
packages/apps/esm-devtools-app/dist/openmrs-esm-devtools-app.js 3.03 kB
packages/apps/esm-implementer-tools-app/dist/199.js 32.8 kB
packages/apps/esm-implementer-tools-app/dist/220.js 4.43 kB
packages/apps/esm-implementer-tools-app/dist/246.js 449 B
packages/apps/esm-implementer-tools-app/dist/263.js 6.66 kB
packages/apps/esm-implementer-tools-app/dist/319.js 595 B
packages/apps/esm-implementer-tools-app/dist/426.js 1.66 kB
packages/apps/esm-implementer-tools-app/dist/466.js 5.85 kB
packages/apps/esm-implementer-tools-app/dist/499.js 2.87 kB
packages/apps/esm-implementer-tools-app/dist/500.js 2.83 kB
packages/apps/esm-implementer-tools-app/dist/519.js 400 B
packages/apps/esm-implementer-tools-app/dist/574.js 560 B
packages/apps/esm-implementer-tools-app/dist/575.js 713 B
packages/apps/esm-implementer-tools-app/dist/587.js 2.81 kB
packages/apps/esm-implementer-tools-app/dist/620.js 126 kB
packages/apps/esm-implementer-tools-app/dist/625.js 562 B
packages/apps/esm-implementer-tools-app/dist/657.js 562 B
packages/apps/esm-implementer-tools-app/dist/735.js 2.63 kB
packages/apps/esm-implementer-tools-app/dist/757.js 560 B
packages/apps/esm-implementer-tools-app/dist/788.js 42.9 kB
packages/apps/esm-implementer-tools-app/dist/807.js 1.44 kB
packages/apps/esm-implementer-tools-app/dist/833.js 681 B
packages/apps/esm-implementer-tools-app/dist/837.js 55.1 kB
packages/apps/esm-implementer-tools-app/dist/889.js 175 kB
packages/apps/esm-implementer-tools-app/dist/941.js 11.7 kB
packages/apps/esm-implementer-tools-app/dist/main.js 4.12 kB
packages/apps/esm-implementer-tools-app/dist/openmrs-esm-implementer-tools-app.js 3.25 kB
packages/apps/esm-login-app/dist/144.js 21.7 kB
packages/apps/esm-login-app/dist/236.js 272 B
packages/apps/esm-login-app/dist/240.js 364 B
packages/apps/esm-login-app/dist/261.js 6.28 kB
packages/apps/esm-login-app/dist/263.js 6.65 kB
packages/apps/esm-login-app/dist/272.js 264 B
packages/apps/esm-login-app/dist/319.js 480 B
packages/apps/esm-login-app/dist/336.js 234 B
packages/apps/esm-login-app/dist/337.js 1.3 kB
packages/apps/esm-login-app/dist/438.js 6 kB
packages/apps/esm-login-app/dist/539.js 298 B
packages/apps/esm-login-app/dist/574.js 428 B
packages/apps/esm-login-app/dist/610.js 5.84 kB
packages/apps/esm-login-app/dist/625.js 421 B
packages/apps/esm-login-app/dist/63.js 16.5 kB
packages/apps/esm-login-app/dist/642.js 10.7 kB
packages/apps/esm-login-app/dist/644.js 305 B
packages/apps/esm-login-app/dist/673.js 284 B
packages/apps/esm-login-app/dist/735.js 2.63 kB
packages/apps/esm-login-app/dist/757.js 509 B
packages/apps/esm-login-app/dist/788.js 42.9 kB
packages/apps/esm-login-app/dist/807.js 685 B
packages/apps/esm-login-app/dist/829.js 10.2 kB
packages/apps/esm-login-app/dist/833.js 528 B
packages/apps/esm-login-app/dist/889.js 175 kB
packages/apps/esm-login-app/dist/967.js 576 B
packages/apps/esm-login-app/dist/main.js 3.48 kB
packages/apps/esm-login-app/dist/openmrs-esm-login-app.js 3.17 kB
packages/apps/esm-offline-tools-app/dist/202.js 9.73 kB
packages/apps/esm-offline-tools-app/dist/206.js 7.41 kB
packages/apps/esm-offline-tools-app/dist/263.js 6.66 kB
packages/apps/esm-offline-tools-app/dist/273.js 48.9 kB
packages/apps/esm-offline-tools-app/dist/281.js 16.3 kB
packages/apps/esm-offline-tools-app/dist/297.js 466 B
packages/apps/esm-offline-tools-app/dist/316.js 2.06 kB
packages/apps/esm-offline-tools-app/dist/319.js 1.04 kB
packages/apps/esm-offline-tools-app/dist/391.js 305 B
packages/apps/esm-offline-tools-app/dist/412.js 1.58 kB
packages/apps/esm-offline-tools-app/dist/47.js 6.34 kB
packages/apps/esm-offline-tools-app/dist/501.js 24.2 kB
packages/apps/esm-offline-tools-app/dist/526.js 15.7 kB
packages/apps/esm-offline-tools-app/dist/538.js 6.79 kB
packages/apps/esm-offline-tools-app/dist/541.js 383 B
packages/apps/esm-offline-tools-app/dist/574.js 1.04 kB
packages/apps/esm-offline-tools-app/dist/594.js 11.4 kB
packages/apps/esm-offline-tools-app/dist/625.js 1.04 kB
packages/apps/esm-offline-tools-app/dist/63.js 16.5 kB
packages/apps/esm-offline-tools-app/dist/696.js 410 B
packages/apps/esm-offline-tools-app/dist/697.js 5.47 kB
packages/apps/esm-offline-tools-app/dist/733.js 290 B
packages/apps/esm-offline-tools-app/dist/735.js 2.63 kB
packages/apps/esm-offline-tools-app/dist/752.js 2.83 kB
packages/apps/esm-offline-tools-app/dist/757.js 1.21 kB
packages/apps/esm-offline-tools-app/dist/78.js 6.15 kB
packages/apps/esm-offline-tools-app/dist/788.js 42.9 kB
packages/apps/esm-offline-tools-app/dist/807.js 1.03 kB
packages/apps/esm-offline-tools-app/dist/833.js 1.23 kB
packages/apps/esm-offline-tools-app/dist/889.js 175 kB
packages/apps/esm-offline-tools-app/dist/937.js 6.61 kB
packages/apps/esm-offline-tools-app/dist/main.js 56.8 kB
packages/apps/esm-offline-tools-app/dist/openmrs-esm-offline-tools-app.js 3.25 kB
packages/apps/esm-primary-navigation-app/dist/241.js 13.9 kB
packages/apps/esm-primary-navigation-app/dist/263.js 6.66 kB
packages/apps/esm-primary-navigation-app/dist/319.js 183 B
packages/apps/esm-primary-navigation-app/dist/574.js 183 B
packages/apps/esm-primary-navigation-app/dist/575.js 441 B
packages/apps/esm-primary-navigation-app/dist/625.js 184 B
packages/apps/esm-primary-navigation-app/dist/63.js 16.5 kB
packages/apps/esm-primary-navigation-app/dist/674.js 680 B
packages/apps/esm-primary-navigation-app/dist/710.js 6.44 kB
packages/apps/esm-primary-navigation-app/dist/716.js 6.12 kB
packages/apps/esm-primary-navigation-app/dist/735.js 2.64 kB
packages/apps/esm-primary-navigation-app/dist/743.js 7.25 kB
packages/apps/esm-primary-navigation-app/dist/757.js 209 B
packages/apps/esm-primary-navigation-app/dist/788.js 42.9 kB
packages/apps/esm-primary-navigation-app/dist/807.js 182 B
packages/apps/esm-primary-navigation-app/dist/833.js 210 B
packages/apps/esm-primary-navigation-app/dist/889.js 175 kB
packages/apps/esm-primary-navigation-app/dist/948.js 3.07 kB
packages/apps/esm-primary-navigation-app/dist/953.js 12.8 kB
packages/apps/esm-primary-navigation-app/dist/main.js 4.78 kB
packages/apps/esm-primary-navigation-app/dist/openmrs-esm-primary-navigation-app.js 3.29 kB
packages/framework/esm-api/dist/openmrs-esm-api.js 9.68 kB
packages/framework/esm-breadcrumbs/dist/openmrs-esm-breadcrumbs.js 2.66 kB
packages/framework/esm-config/dist/openmrs-esm-module-config.js 7.87 kB
packages/framework/esm-dynamic-loading/dist/openmrs-esm-dynamic-loading.js 2.12 kB
packages/framework/esm-error-handling/dist/openmrs-esm-error-handling.js 894 B
packages/framework/esm-extensions/dist/openmrs-esm-extensions.js 8.05 kB
packages/framework/esm-feature-flags/dist/openmrs-esm-feature-flags.js 1.67 kB
packages/framework/esm-framework/dist/146.openmrs-esm-framework.js 6.69 kB
packages/framework/esm-framework/dist/530.openmrs-esm-framework.js 2.92 kB
packages/framework/esm-framework/dist/645.openmrs-esm-framework.js 9.31 kB
packages/framework/esm-framework/dist/710.openmrs-esm-framework.js 6.48 kB
packages/framework/esm-framework/dist/735.openmrs-esm-framework.js 2.66 kB
packages/framework/esm-framework/dist/788.openmrs-esm-framework.js 42.9 kB
packages/framework/esm-framework/dist/openmrs-esm-framework.js 354 kB
packages/framework/esm-globals/dist/openmrs-esm-globals.js 714 B
packages/framework/esm-offline/dist/openmrs-esm-offline.js 34.4 kB
packages/framework/esm-react-utils/dist/openmrs-esm-react-utils.js 18.5 kB
packages/framework/esm-state/dist/openmrs-esm-state.js 888 B
packages/framework/esm-styleguide/dist/openmrs-esm-styleguide.js 82.6 kB
packages/framework/esm-utils/dist/openmrs-esm-utils.js 10.2 kB
packages/shell/esm-app-shell/dist/972cf9cf70317f32.js 1.58 kB
packages/shell/esm-app-shell/dist/e861b352d93d75e6.js 9.88 kB
packages/shell/esm-app-shell/dist/f74c709ff5e0af39.js 6.04 kB
packages/shell/esm-app-shell/dist/openmrs.2667f4ab4311df42.js 323 kB
packages/shell/esm-app-shell/dist/service-worker.js 57.3 kB
packages/tooling/openmrs/dist/cli.js 3.08 kB
packages/tooling/openmrs/dist/commands/assemble.js 2.73 kB
packages/tooling/openmrs/dist/commands/build.js 1.61 kB
packages/tooling/openmrs/dist/commands/debug.js 579 B
packages/tooling/openmrs/dist/commands/develop.js 1.84 kB
packages/tooling/openmrs/dist/commands/index.js 438 B
packages/tooling/openmrs/dist/commands/start.js 848 B
packages/tooling/openmrs/dist/index.js 517 B
packages/tooling/openmrs/dist/runner.js 870 B
packages/tooling/openmrs/dist/utils/config.js 726 B
packages/tooling/openmrs/dist/utils/debugger.js 529 B
packages/tooling/openmrs/dist/utils/dependencies.js 639 B
packages/tooling/openmrs/dist/utils/helpers.js 393 B
packages/tooling/openmrs/dist/utils/importmap.js 3.25 kB
packages/tooling/openmrs/dist/utils/index.js 444 B
packages/tooling/openmrs/dist/utils/logger.js 234 B
packages/tooling/openmrs/dist/utils/npmConfig.js 854 B
packages/tooling/openmrs/dist/utils/untar.js 721 B
packages/tooling/openmrs/dist/utils/variables.js 192 B
packages/tooling/openmrs/dist/utils/webpack.js 276 B
packages/tooling/webpack-config/dist/index.js 3.24 kB

compressed-size-action

@denniskigen denniskigen marked this pull request as draft September 26, 2023 12:58
@denniskigen
Copy link
Member Author

denniskigen commented Sep 26, 2023

It looks like the canonical way to handle the height issue is as shown here.

@denniskigen
Copy link
Member Author

denniskigen commented Sep 26, 2023

Fix ContentSwitcher height

Height fixed

@denniskigen denniskigen marked this pull request as ready for review September 26, 2023 19:16
@denniskigen denniskigen merged commit 9546c69 into main Sep 26, 2023
@denniskigen denniskigen deleted the fix/content-switcher-style-overrides branch September 26, 2023 20:22
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.

2 participants