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.

Sign up for GitHub

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

Jump to bottom

Modal: full feature support with Dimmer and docs #383

Merged
merged 6 commits into from
Aug 13, 2016
Merged

Modal: full feature support with Dimmer and docs #383

merged 6 commits into from
Aug 13, 2016

Conversation

levithomason
Copy link
Member

@levithomason levithomason commented Aug 9, 2016 •
edited
Loading

An image content Modal requires the image to be a div.ui.image > img. However, some components, like the List, require markup of img.ui.image.

Image updates in #280 provide a wrapped prop for choosing the div wrapped markup variation.

Updates

Fixes #175 #84. This PR adds many features and full docs for the Modal. Added:

  • missing ModalDescription
  • portal render to body
  • optional configurable dimmer
  • auto scrolling when height is larger than window
  • close on esc (fixes Modal Keyboard Event Handlers #175)
  • close on click outside
  • callbacks onHide, onShow

@codecov-io
Copy link

codecov-io commented Aug 9, 2016 •
edited
Loading

Current coverage is 94.38% (diff: 100%)

Merging #383 into master will increase coverage by 0.53%

@@             master       #383   diff @@
==========================================
  Files            74         75     +1   
  Lines           960       1016    +56   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            901        959    +58   
+ Misses           59         57     -2   
  Partials          0          0

Powered by Codecov. Last update 012cba6...5846198

@levithomason levithomason force-pushed the modal-updates branch 2 times, most recently from e6e287f to a2bd945 Compare August 11, 2016 07:03
levithomason
levithomason reviewed Aug 11, 2016
View reviewed changes
config.js
@@ -61,7 +61,6 @@ config = Object.assign({}, config, {
compiler_output_path: paths.base(config.dir_docs_dist),
compiler_public_path: __BASE__ || '/',
compiler_vendor: [
'bluebird',
Copy link
Member Author

Choose a reason for hiding this comment

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

🎉

This was referenced Aug 11, 2016
Merged
Merged
@levithomason levithomason force-pushed the modal-updates branch 2 times, most recently from f3d5701 to 1dc4409 Compare August 13, 2016 12:09
@tslater
Copy link
Contributor

tslater commented Aug 13, 2016

Will this let me have a dimmer/dimmed background? I can't figure out how to do that currently.

@levithomason
Copy link
Member Author

Yes sir it will! It includes every variation of the dimmer as well, even false for none. There are also complete docs for this.

@tslater
Copy link
Contributor

tslater commented Aug 13, 2016

That will be awesome! Nice work!

@levithomason
Copy link
Member Author

Just pushed 100% test coverage, will merge and release if it passes.

@levithomason levithomason changed the title Modal docs and features Add Modal Dimmer with full feature support and docs Aug 13, 2016
@levithomason levithomason changed the title Add Modal Dimmer with full feature support and docs Add full Modal feature support with Dimmer and docs Aug 13, 2016
@levithomason levithomason merged commit 89d9a58 into master Aug 13, 2016
@levithomason levithomason deleted the modal-updates branch August 13, 2016 20:46
@levithomason
Copy link
Member Author

@tslater I just released this in stardust@0.33.0. The doc site should be updated by CI in just a few minutes. Once it is, you can see the examples here:

http://technologyadvice.github.io/stardust/modules/modal

@tslater
Copy link
Contributor

tslater commented Aug 13, 2016 •
edited
Loading

I just tested it out and it works great!

I did have to run npm i --save semantic-ui-css...because it said it was missing a package semantic-ui-css/something-or-other-about-forms.

The update also solved an issue where I had to add a top style to my modal, I deleted the rule and things are working great. Thanks so much!

@levithomason
Copy link
Member Author

Thanks for the report. There is one remaining jQuery plugin, form validation. We are working to remove it, see #247. I removed the semantic-ui-css dep which was a mistake as it also included the jQuery plugin. Just released a patch in v0.34.1.

Glad to hear it is solving problems for you as well!

@tslater
Copy link
Contributor

tslater commented Aug 14, 2016

I just upgraded to 0.34.1 and removed semantic-ui-css. It's working great!

layershifter pushed a commit that referenced this pull request Aug 14, 2016
@levithomason @layershifter
Add full Modal feature support with Dimmer and docs (#383)
a20bfd2 
* chore(package): add react-portal, update sui-css and simulant

* feat(Modal): add dimmer, support all features

* test(Modal): add and update tests

* docs(Modal): add Modal docs

* refactor(Confirm): update confirm

* test(Confirm): test updated confirm
@levithomason levithomason mentioned this pull request Aug 14, 2016
Closed
@levithomason levithomason changed the title Add full Modal feature support with Dimmer and docs Modal: full feature support with Dimmer and docs Aug 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Modal Keyboard Event Handlers
3 participants
@levithomason @codecov-io @tslater