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

feat(common): add animation for collapse and accordion components #5146

Merged
merged 11 commits into from
Apr 19, 2019
Merged

feat(common): add animation for collapse and accordion components #5146

merged 11 commits into from
Apr 19, 2019

Conversation

Domainv
Copy link
Contributor

@Domainv Domainv commented Apr 3, 2019 •
edited
Loading

PR Checklist

Before creating new PR, please take a look at checklist below to make sure that you've done everything that needs to be done before we can merge it.

  • read and followed the CONTRIBUTING.md guide.
  • built and tested the changes locally.
  • added/updated tests.
  • added/updated API documentation.
  • added/updated demos.

@ghost ghost assigned Domainv Apr 3, 2019
@ghost ghost added the needs review label Apr 3, 2019
@Domainv Domainv changed the title feat(common): add animations WIP feat(common): add animations Apr 3, 2019
@Domainv Domainv added the WIP label Apr 3, 2019
@Domainv Domainv force-pushed the feat-animation branch 2 times, most recently from b474eae to 09748b5 Compare April 4, 2019 12:31
@codecov
Copy link

codecov bot commented Apr 10, 2019

Codecov Report

Merging #5146 into development will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           development    #5146   +/-   ##
============================================
  Coverage        51.51%   51.51%           
============================================
  Files                3        3           
  Lines               99       99           
  Branches            17       17           
============================================
  Hits                51       51           
  Misses              38       38           
  Partials            10       10

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0006146...977aa3f. Read the comment docs.

@codecov
Copy link

codecov bot commented Apr 10, 2019 •
edited
Loading

Codecov Report

Merging #5146 into development will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           development    #5146   +/-   ##
============================================
  Coverage        51.51%   51.51%           
============================================
  Files                3        3           
  Lines               99       99           
  Branches            17       17           
============================================
  Hits                51       51           
  Misses              38       38           
  Partials            10       10

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f5be2db...15ea581. Read the comment docs.

@Domainv Domainv changed the title WIP feat(common): add animations feat(common): add animations Apr 11, 2019
@ghost ghost added the needs review label Apr 11, 2019
@Domainv Domainv changed the title feat(common): add animations feat(common): add animation for collapse and accordion components Apr 11, 2019
@Domainv Domainv added this to the 4.2.1 milestone Apr 12, 2019
@ludmilanesvitiy
Copy link
Contributor

Screenshot from 2019-04-12 15-37-39
This is a view from Edge 41, and same in IE11
Peek 2019-04-12 15-39

@ludmilanesvitiy
Copy link
Contributor

  1. Need to investigate: previously we had a .show class name for collapse, which was open. But now this class missed.
    Screenshot from 2019-04-12 16-16-37
    Screenshot from 2019-04-12 16-16-22

@ghost ghost added needs review and removed needs fix labels Apr 16, 2019
@ludmilanesvitiy
Copy link
Contributor

Collapse with animation works well.
Found 1 issue with accordion on IE11/Edge, when 4th bootstrap version:
After click on Accordion item, it doesn't open:
Peek 2019-04-17 10-34

@ghost ghost added needs review and removed needs fix labels Apr 17, 2019
@ludmilanesvitiy
Copy link
Contributor

Animation tested, in Chrome latest(+mobile, + tablet), in FF latest, in Edge(win10), in IE(win81) - looks good. Tested bs3, bs4. Ready for merge.

@Domainv Domainv merged commit 191e5b4 into development Apr 19, 2019
@ghost ghost removed the ready for merge label Apr 19, 2019
@delete-merged-branch delete-merged-branch bot deleted the feat-animation branch April 19, 2019 11:07
leo6104 pushed a commit to leo6104/ngx-bootstrap that referenced this pull request Oct 10, 2019
@Domainv @leo6104
feat(common): add animation for collapse and accordion components (va…
d241487 
…lor-software#5146)

* feat(common): add animation to collapse and accordion
IraErshova pushed a commit to IraErshova/ngx-bootstrap that referenced this pull request Jan 20, 2020
@Domainv @IraErshova
feat(common): add animation for collapse and accordion components (va…
051dbf0 
…lor-software#5146)

* feat(common): add animation to collapse and accordion
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Domainv Domainv

Labels
comp(accordion) comp(collapse)
Projects
None yet
Milestone
4.2.0
Development

Successfully merging this pull request may close these issues.
2 participants
@Domainv @ludmilanesvitiy