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

feat: upgrade to fundamental-styles 0.2.0 #742

Merged
merged 34 commits into from
Sep 17, 2019
Merged

feat: upgrade to fundamental-styles 0.2.0 #742

merged 34 commits into from
Sep 17, 2019

Conversation

jbadan
Copy link
Contributor

@jbadan jbadan commented Sep 13, 2019

Description

KNOWN ISSUES out of scope of this pr:

  1. follow ups in FS: https://github.com/SAP/fundamental-styles/wiki/0.2.0-followup-needed
  2. Popover arrow is not showing up at all
  3. Popover implementation library styles solutions
    • ComboBox, MultiInput, DatePicker, Timepicker, SearchInput: width of overlay
    • DatePicker: Calendar needs padding inside the Popover body
    • TimePicker: padding for Time
  4. further followups in FR: https://github.com/SAP/fundamental-react/wiki/Fundamental-styles-0.2.0-followup
  5. dynamic imports - prop based
  6. Shellbar SearchInput is open always - we need to rewrite a ton of logic to get this to be hidden smoothly.

[BREAKING CHANGE]

Infrastructure

  1. Consumers will need their webpack configurations to handle css

Components

Changes besides classnames listed here: https://github.com/SAP/fundamental-styles/wiki/Breaking-Changes

  1. ActionBar
    • removed mobile, width props
  2. Button
    • removed dropdown, navbar props
    • removed fd-dropdown__control, fd-global-nav__btn classes
  3. Calendar
    • removed fd-button--standard class from buttons
  4. ComboBoxInput
    • removed fd-combobox-control class + div
  5. FormItem
    • removed fd-form-item--check
    • removed isCheck prop
    • replaced FormItem checkbox with Checkbox component
  6. InputGroup
    • removed "search" type - use SearchInput
    • removed searchButtonProps prop
  7. ListGroupItemCheckbox
    • replaced internals with Checkbox component
  8. MenuItem
    • removed separator prop (now lives in MenuList)
  9. Modal
    • removed extraneous div around actions
  10. MultiInput
    • removed extraneous div around popover control
  11. SearchInput
    • searchList prop propTypes is more explicit - requires array of objects with keys including text (required) and callback
  12. Shellbar
    • removed several extraneous divs
  13. Tile
    • removed rowSpan, columnSpan (handled in LayoutGrid)
    • removed colorAccent,backgroundColor (handled by helper classes)
  14. TimePicker
    • removed extraneous div

Removed Components:

  1. PanelGrid
  2. TileGrid

New Components:

  1. Checkbox
  2. Link
  3. LayoutGrid

fixes #issueid

@netlify
Copy link

netlify bot commented Sep 13, 2019

Deploy preview for fundamental-react ready!

Built with commit 7b7a89e

https://deploy-preview-742--fundamental-react.netlify.com

Copy link
Contributor

@greg-a-smith greg-a-smith left a comment

Choose a reason for hiding this comment

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

This is just a partial collection of comments from the first files down through Dropdown. More to come, but trying to keep the review moving.

src/Alert/Alert.js Show resolved Hide resolved
src/Alert/Alert.Component.js Outdated Show resolved Hide resolved
src/Badge/Counter.js Outdated Show resolved Hide resolved
src/Badge/Status.js Outdated Show resolved Hide resolved
src/Calendar/Calendar.Component.js Show resolved Hide resolved
src/ComboboxInput/ComboboxInput.js Show resolved Hide resolved
src/DatePicker/DatePicker.js Show resolved Hide resolved
src/Dropdown/Dropdown.Component.js Show resolved Hide resolved
src/Dropdown/Dropdown.test.js Outdated Show resolved Hide resolved
@CLAassistant
Copy link

CLAassistant commented Sep 17, 2019

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ jbadan
✅ bcullman
❌ greg-a-smith
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Contributor

@greg-a-smith greg-a-smith left a comment

Choose a reason for hiding this comment

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

Another Herculean effort 💪 @jbadan! There are a few minor things I found here and there, but let's get this PR merged and do some smaller-scoped follow-up work to address those. I will add my findings to the wiki pages already created.

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.

None yet

3 participants