Skip to content

V5#574

Merged
TheSharpieOne merged 35 commits intomasterfrom
v5
Oct 2, 2017
Merged

V5#574
TheSharpieOne merged 35 commits intomasterfrom
v5

Conversation

@TheSharpieOne
Copy link
Copy Markdown
Member

@TheSharpieOne TheSharpieOne commented Sep 16, 2017

Anyone and everyone please review and give feedback. Lots of changes and descriptive commit messages with deprecations and breaking changes information (which I hope get automatically added to the release notes).

Tests have been updated
Docs have been (mostly) updated.

closes #487, closes #529, closes #539, closes #113, closes #337, closes #424, closes #465, closes #438, closes #466, closes #502, closes #532, closes #535, closes #563, closes #548, closes #551, closes #485, closes #583
(Have to put keyword in front of every issue number... why can't it just be comma seprated)

TheSharpieOne and others added 18 commits August 16, 2017 13:08
Add deprected util to mark prop-types as deprected
Add warnOnce to display a message in the console, never the same message twice
Add CardBody, depercating CardBlock
Depercated Card's block prop in favor of new body prop
inverse now add .text-white instead of .card-inverse
outline is now border-{color} instead of card-outline-{color}
color is now bg-{color} instead of card-{color}
outline still uses color's value
update examples
update tests
Add .show to DropdownMenu base on context.isOpen
Remove .active from DropdownToggle to match bootstrap
Update tests
Navbar deprecated inverse in favor of dark
Narbar added expandable as alais to toggleable since class name has change
Navbar changed toggler class to use expand
Navbar removed full as it is no longer a thing
NavbarToggler remove left and right as they are no longer a thing
Nav change vertical to allow for string breakpoint (sm,md,lg,etc) .flex-{vertical}-column
Nav add horizontal to trigger .justift-content-{horizontal} utility classes
Nav add card prop to trigger with tabs/pills props to make .card-header-tabs and .card-header-pills respectfully
Nav add fill prop to trigger nav-fill class
fix some examples/docs (removing props which no longer exist) and adding new props to the lists (still needs additonal new examples)
Update tests
Deprecate PopoverTitle in favor of PopoverHeader
Deprecate PopoverContent in favor of PopoverBody
Update docs/examples
Update tests
Check if the dialog has a parent which can receive focus before trying to focus it.
This workaround will allow react 16 to use Modal without forcing autoFocus={false}
Add tests
Update existing tests (jest has been upgraded) 
improve code coverage 
Breaking change: getRef has been renamed to innerRef to line up with other libraries.
Breaking change: tether props have been removed.
Fixed Error 'placements' is not exported by node_modules/popper.js/dist/esm/popper.js
Renamed Popper to PopperContent in Tooltip.js to match Popover.js.
Update Alert documentation to stay up to date with bootstrap v4 beta.
Add light and dark colors and add examples with Link color and
Additional content.
Remove default badge and update documentation so it's closer to bootstrap v4 beta
docs
* Updated rollup and webpack configs

Removed global 'react-transition-group' from both rollup and webpack configs

* Updated Alert for [email protected]

Also updated props related to transitions in order to better match updated react-transition-group props

* Updated AlertPage.js docs for new transition props

* Fixed Alert tests for new transition props

* Fixed lint issues related to Alert changes

* Almost fixed Fade and Modal to work with [email protected]

* Fixed Modal exit transition

* Fixed lint issues

* Fixed Fade test

* Fixed Modal test; removed use of setImmediate

* Fixed Modal.spec.js formatting

* Updated install docs to remove react-transition-group

* Updated modal documentation

* Almost got carousel to work

* Fixed fade props passing

* Changed the way Carousel works; still not quite fixed

* More attempts at fixing Carousel

* Changed Alert to use Fade

* Fixed all react-transition-group imports to not cherry pick

* Now modal is broken :/ Fixing this

* Some mork work

* Changed Transition uses to use constants instead of strings

* Made Carousel work! Plus added Carousel slide prop

* Minor change to Fade test

* Removed debug logging from CarouselItem

* Fixed Modal and siplified logic

* Extracted transition timeouts to utils

* Simplified CarouselItem transition logic

* Removed unnecessary noop props (they exist in Transition.defaultProps)

* Removed CarouselItem status field

* Added docs for Fade

This meant adding  FadePage, plus adding to routes and navigation. I also updated prism to add the autolinker plugin to enable links in the docs for proptypes

* Updated Alert and Modal docs to point to Fade docs

* Added mountOnEnter for Modal backdrop

* Updated carousel proptypes doc

* Fixed eslint issues

* Updated Collapse to use Transition

* Removed confusing callbacks from Modal. The callbacks on modalTransition can be used instead

* Updated react-transition-group in package.json

* Fixed collapse tests

* Fixed eslint issues

* Fixed Collapse callback issue, updated test, updated examples

* Updated Collapse Docs

* Added some Alert tests

* Updated Collapse and Collapse tests

* Updated Carousel tests

* Updated Fade tests

* Fixed modal tests
Updated tests
Updated docs
fixes dropdowns, tooltips, and popovers not triggers on touch events

Closes #456 #458
Added/updated test
Added/updated docs

deprecation: the state prop on input has now become the valid prop which accepts a boolean
breaking change: The color prop has been removed from FromGroup, see the valid prop on Input.
@TheSharpieOne TheSharpieOne added this to the v5 milestone Sep 16, 2017
@th3fallen
Copy link
Copy Markdown

🚢 it :)

@supergibbs
Copy link
Copy Markdown
Collaborator

Ship it but as an beta; you'll get much more testing when devs can easily try it in their apps.

Comment thread src/Popover.js Outdated
const propTypes = {
placement: PropTypes.oneOf(tetherAttachements),
target: PropTypes.string.isRequired,
placement: PropTypes.oneOf(PopperJS.placements),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@TheSharpieOne looks like the Popover component is missing the container prop which is in the v5 docs.

@SimeonC
Copy link
Copy Markdown

SimeonC commented Sep 20, 2017

EDIT: Use npm install reactstrap@next as below now!

For those who want to test I have a branch with the lib files commited in here: https://github.com/SimeonC/reactstrap/tree/v5.

You can install via npm install simeonc/reactstrap#v5. I'll try keep it updated alongside this branch until they release something more official.

Comment thread src/Popover.js Outdated
const attributes = omit(this.props, Object.keys(propTypes));
const classes = mapToCssModules(classNames(
'popover-inner',
this.props.className
Copy link
Copy Markdown

@SimeonC SimeonC Sep 20, 2017

Choose a reason for hiding this comment

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

Shouldn't this.props.className be on the popperClasses instead?
Currently I get the following scenario:

<Popover
    className="day-picker-popover"
    placement="bottom"
    isOpen={open}
>

Outputs:

<div data-reactroot="" class="popover show bs-popover-bottom" style="position: absolute; transform: translate3d(176px, 489px, 0px); top: 0px; left: 0px; will-change: transform;" data-placement="bottom">
    <div class="popover-inner day-picker-popover">

I expected "day-picker-popover" to be on the parent element not the inner as I can't use the classname to override popover styles in this case.

Copy link
Copy Markdown
Member Author

@TheSharpieOne TheSharpieOne Sep 20, 2017

Choose a reason for hiding this comment

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

Yeah, it probably makes more sense to have that apply to the popper and create a new prop to extend the inner popover classes.

@ghost
Copy link
Copy Markdown

ghost commented Sep 20, 2017

Navbar toggleable true is not the expected.
I saw in the code a kind of fallback you can use toggleable or expandable
https://github.com/SimeonC/reactstrap/blob/v5/src/Navbar.js#L57

If I use toggleable={true}, it will add the class expand. I am not sure but I think on the beta version it means always expanded

Deprecate static prop in favor of plaintext prop. Use plaintaxt prop in the same way (boolean)

closes #485
Now when menu is not 'open' it will not be a popper

Closes #584
TheSharpieOne and others added 3 commits September 22, 2017 15:00
Adds the ability to control/navigate the dropdown and it's menu using the keyboard (space, arrow, esc keys)
This mimic bootstrap 4's functionality

Closes #580
@TheSharpieOne
Copy link
Copy Markdown
Member Author

[email protected] has been published to npm using the next tag. It can be installed using npm install reactstrap@next
5.0.0-alpha.1 includes performance fix for Dropdown, Dropdown keyboard navigation/control, and plaintext prop on Input (deprecating the static prop)

@med1sh
Copy link
Copy Markdown

med1sh commented Sep 27, 2017

Hi, I use warning, danger, success, and muted states. I also like the feature of highlighting the label text for where the field is in a display state. Can we get that ability added back? the valid prop only has 3 states.

Thanks

@supergibbs
Copy link
Copy Markdown
Collaborator

supergibbs commented Sep 27, 2017

Which component do you mean? You can always add bg-<state> or text-<state> to className on any component. FYI, I think muted was replaced with light

@med1sh
Copy link
Copy Markdown

med1sh commented Sep 27, 2017

@supergibbs You can see the demonstrated changes here: 8b2386f#diff-bc5250a5d5097846d70ffc59fd9a3769

@supergibbs
Copy link
Copy Markdown
Collaborator

@med1sh I see, that looks more like a Bootstrap decision. Take a look at the history for _forms.scss on June 6th.

@TheSharpieOne
Copy link
Copy Markdown
Member Author

TheSharpieOne commented Sep 27, 2017

@supergibbs is correct. Bootstrap made this change so reactstrap followed suit.
They completely changed validation, most of it is automatically/handled by css. The props reactstrap has trigger the "server-side" aspect, essentially forcing the validation to display.
https://getbootstrap.com/docs/4.0/components/forms/#server-side

Also, .text-muted is still used on bootstrap's forms documentation though it is no longer in the color utilities docs. Looks like it might be replaced by .text-secondary but I am not sure.

@med1sh
Copy link
Copy Markdown

med1sh commented Sep 27, 2017

Ok, cool. Not sure how much I like that change. Thanks

@TheSharpieOne
Copy link
Copy Markdown
Member Author

@med1sh Yeah, I am not a big fan of it either, but it make sense with the change to use the CSS validation pseudo class which only have the two states (valid and invalid).

Also, it looks like .text-muted is here to stay.

@autarch
Copy link
Copy Markdown

autarch commented Sep 28, 2017

FWIW, with both this branch and the 5.0.0-alpha.1 tag, I cannot build the docs locally. I tried running yarn install followed by yarn run build-docs and I get this:

$  yarn run build-docs
yarn run v1.1.0
$ cross-env WEBPACK_BUILD=production webpack --config ./webpack.dev.config.js --progress --colors
clean-webpack-plugin: /home/autarch/projects/reference/reactstrap/build has been removed.
Hash: f2fe324d8c78773def51  
Version: webpack 1.15.0
Child
    Hash: f2fe324d8c78773def51
    Version: webpack 1.15.0
    Time: 578ms
        + 2 hidden modules
    
    ERROR in bundle.js from UglifyJs
    SyntaxError: Unexpected token: string (bootstrap-css) [./docs/lib/app.js:1,7]
Done in 1.31s.

This is pretty frustrating since there's no other way to see the docs AFAICT (cue a rant about how every package should include its full docs and npmjs.com should host said docs, but that's getting pretty OT ;)

@TheSharpieOne
Copy link
Copy Markdown
Member Author

TheSharpieOne commented Sep 28, 2017

@autarch I need @eddywashere to allow netlify permission to the repo, but it has a really awesome feature which automatically builds and publishes things (such as docs) with every PR. (super handy right?)
I am getting that same thing when doing build-docs (with npm), but you can use the start cmd/script to start the local "dev" instance which just compiles, watches, and serves the docs to play around locally. I'll look into the build-docs not working.

The build-docs script was missing BABEL_ENV=webpack which tells babel to transform the imports. It doesn't transform them by default since the rollup build does esm and need the imports as is. I pushed up the change to v5.

@cr101
Copy link
Copy Markdown

cr101 commented Oct 1, 2017

Will V5 work with React 16?

@autarch
Copy link
Copy Markdown

autarch commented Oct 1, 2017

I've used both v4 and the v5 alpha with React 16.

@virgofx
Copy link
Copy Markdown
Collaborator

virgofx commented Oct 1, 2017

It's mostly compatible with R16; however, there are a few issues with some of the components. A few issues have already been created.

@TheSharpieOne TheSharpieOne merged commit 72fb714 into master Oct 2, 2017
@TheSharpieOne TheSharpieOne deleted the v5 branch October 2, 2017 03:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet