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: (Core|Platform) bring v.0.14.0 of fundamental-styles #4116

Merged
merged 3 commits into from
Dec 23, 2020

Conversation

InnaAtanasova
Copy link
Contributor

@InnaAtanasova InnaAtanasova commented Dec 15, 2020

Please provide a brief summary of this pull request.

This PR is bringing version 0.14.0 of Fundamental-styles.
Changes I made:

  • removed custom styling for Flexible column layout. Now it's importing the styling from Fundamental-styles
  • removed fd-dynamic-page__collapsible-header-container--collapsed class which was removed in Fundamental-styles
  • updated Switch component to use the latest class naming
  • re-structured Switch component to reflect the latest changes from Fundamental-styles, added a span with optional text for active/inactive state

BREAKING CHANGE:

  • input property optionalText for Switch has been removed

Please check whether the PR fulfills the following requirements

Documentation checklist:

@InnaAtanasova InnaAtanasova added platform platform core Core library specific issues denoland labels Dec 15, 2020
@InnaAtanasova InnaAtanasova added this to the Sprint 52 - Hilo milestone Dec 15, 2020
@InnaAtanasova InnaAtanasova requested a review from a team December 15, 2020 21:17
@InnaAtanasova InnaAtanasova self-assigned this Dec 15, 2020
@netlify
Copy link

netlify bot commented Dec 15, 2020

✔️ Deploy preview for fundamental-ngx ready!

🔨 Explore the source changes: 03fd699

🔍 Inspect the deploy logs: https://app.netlify.com/sites/fundamental-ngx/deploys/5fdcf42d18f1220008e6946d

😎 Browse the preview: https://deploy-preview-4116--fundamental-ngx.netlify.app

@InnaAtanasova InnaAtanasova force-pushed the fix/remove-custom-styling-flexible-column branch 2 times, most recently from 4d2ebe9 to df5aa65 Compare December 15, 2020 21:38
@mikerodonnell89
Copy link
Member

Message box close button is black (used to be blue) and has this weird focus outline

Screen Shot 2020-12-15 at 3 43 57 PM

@InnaAtanasova InnaAtanasova force-pushed the fix/remove-custom-styling-flexible-column branch from df5aa65 to 3f0c253 Compare December 16, 2020 16:04
@InnaAtanasova
Copy link
Contributor Author

Message box close button is black (used to be blue) and has this weird focus outline

Screen Shot 2020-12-15 at 3 43 57 PM

The button should not be there per specs. The change is out of the scope of this PR and will be fixed in another.

JKMarkowski
JKMarkowski previously approved these changes Dec 17, 2020
Copy link
Contributor

@JKMarkowski JKMarkowski left a comment

Choose a reason for hiding this comment

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

LGTM. Could you add removal of @Input() optionalText in switch componennt into breaking changes?

Copy link
Contributor

@JKMarkowski JKMarkowski left a comment

Choose a reason for hiding this comment

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

Could you also rebase it to latest master?

  • Action sheet got buttons issue with center
  • Tabs now have shadow, maybe it should be removed in documentation (not lib)
    image

@JKMarkowski
Copy link
Contributor

Should split button be centered in this case?
split button

@InnaAtanasova InnaAtanasova force-pushed the fix/remove-custom-styling-flexible-column branch from 3f0c253 to 11748ac Compare December 17, 2020 14:12
@InnaAtanasova
Copy link
Contributor Author

InnaAtanasova commented Dec 17, 2020

Hi @JKMarkowski the issues with the buttons in Action sheet are due to custom styling for button in ngx. I removed it and now it looks ok. Could you please check?

As per the Split Button, the issue is there before bringing 0.14.0 and is not in the scope of this PR.
I opened an issue for this: SAP/fundamental-styles#1981

Copy link
Contributor

@JKMarkowski JKMarkowski left a comment

Choose a reason for hiding this comment

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

LGTM. Great job!

@JKMarkowski JKMarkowski changed the base branch from master to main December 18, 2020 17:25
@JKMarkowski JKMarkowski merged commit 848bffe into main Dec 23, 2020
@JKMarkowski JKMarkowski deleted the fix/remove-custom-styling-flexible-column branch December 23, 2020 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core library specific issues denoland platform platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants