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

fix(timepicker): enable_disable timepicker in reactive forms #4563

Merged
merged 4 commits into from
Dec 28, 2018
Merged

fix(timepicker): enable_disable timepicker in reactive forms #4563

merged 4 commits into from
Dec 28, 2018

Conversation

FrancescoBorzi
Copy link
Member

@FrancescoBorzi FrancescoBorzi commented Aug 22, 2018 •
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.

This PR fixes: #4055

@ghost ghost added the needs review label Aug 22, 2018
@FrancescoBorzi FrancescoBorzi mentioned this pull request Aug 22, 2018
Closed
@codecov
Copy link

codecov bot commented Aug 22, 2018 •
edited
Loading

Codecov Report

Merging #4563 into development will increase coverage by 0.02%.
The diff coverage is 50%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #4563      +/-   ##
===============================================
+ Coverage        74.79%   74.82%   +0.02%     
===============================================
  Files              277      277              
  Lines             8434     8436       +2     
  Branches          1602     1603       +1     
===============================================
+ Hits              6308     6312       +4     
- Misses            1678     1681       +3     
+ Partials           448      443       -5
Impacted Files Coverage Δ
src/timepicker/timepicker.component.ts 93.91% <50%> (-0.61%) ⬇️
src/chronos/i18n/pl.ts 71.79% <0%> (-2.57%) ⬇️
src/chronos/i18n/sk.ts 80.85% <0%> (-2.13%) ⬇️
src/datepicker/reducer/bs-datepicker.effects.ts 72.47% <0%> (-1.84%) ⬇️
src/chronos/i18n/cs.ts 84.74% <0%> (-1.7%) ⬇️
src/typeahead/typeahead.directive.ts 76.53% <0%> (+1.02%) ⬆️
src/chronos/utils/date-getters.ts 81.81% <0%> (+2.27%) ⬆️
src/chronos/i18n/uk.ts 80.48% <0%> (+2.43%) ⬆️
src/datepicker/engine/flag-days-calendar.ts 81.25% <0%> (+6.25%) ⬆️
src/chronos/i18n/it.ts 100% <0%> (+28.57%) ⬆️

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 9fe4acc...5a5e8a6. Read the comment docs.

@valorkin valorkin added this to the 3.1.4 milestone Dec 16, 2018
valorkin
valorkin previously approved these changes Dec 16, 2018
View reviewed changes
@ghost ghost assigned valorkin Dec 16, 2018
@valorkin valorkin changed the title Fix #4055 fix(timepicker): enable_disable timepicker in reactive forms Dec 16, 2018
@ludmilanesvitiy
Copy link
Contributor

screenshot from 2018-12-17 12-34-24
@FrancescoBorzi @valorkin @Domainv Please, need fix prod build

@FrancescoBorzi
Copy link
Member Author

@ludmilanesvitiy @valorkin thanks for checking my PR, however this is a bit old (opened 4 months ago) and it possibly needs to be merged with master first.

About the pipeline failure, I'm currently on holiday with limited access so I cannot fix it now.

@ghost ghost added needs review and removed needs fix labels Dec 17, 2018
@ludmilanesvitiy
Copy link
Contributor

I've updated this PR, but issue with prod build should be fixed before testing.

@FrancescoBorzi
Copy link
Member Author

@ludmilanesvitiy @valorkin @Domainv

I've fixed the issue with the build. Now the build is still failing but I think it might not be related to my changes. Can you please have a look?

@ludmilanesvitiy
Copy link
Contributor

Tested in Chrome, FF - latest, timepicker looks good.

valorkin
valorkin approved these changes Dec 28, 2018
View reviewed changes
Copy link
Member

@valorkin valorkin left a comment

Choose a reason for hiding this comment

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

Thanks

@valorkin valorkin merged commit ac55b08 into valor-software:development Dec 28, 2018
@ghost ghost removed the ready for merge label Dec 28, 2018
@FrancescoBorzi FrancescoBorzi deleted the issue-4055 branch December 28, 2018 12:32
@FrancescoBorzi
Copy link
Member Author

was this included in 3.1.4 or will be part of 3.1.5 ?

@valorkin
Copy link
Member

Hey, yes. I have merged it just after 3.1.4 release commit )

@FrancescoBorzi
Copy link
Member Author

ok then the milestone should be changed

@Domainv Domainv modified the milestones: 3.1.4, 3.1.5 Jan 4, 2019
leo6104 pushed a commit to leo6104/ngx-bootstrap that referenced this pull request Oct 10, 2019
@FrancescoBorzi @leo6104
fix(timepicker): enable_disable timepicker in reactive forms (valor-s…
f44c256 
…oftware#4563)

* Fix valor-software#4055

* Fix prod build
IraErshova pushed a commit to IraErshova/ngx-bootstrap that referenced this pull request Jan 20, 2020
@FrancescoBorzi @IraErshova
fix(timepicker): enable_disable timepicker in reactive forms (valor-s…
227ede3 
…oftware#4563)

* Fix valor-software#4055

* Fix prod build
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@valorkin valorkin valorkin approved these changes

Assignees

@valorkin valorkin

@ludmilanesvitiy ludmilanesvitiy

Labels
comp(timepicker) needs review
Projects
None yet
Milestone
3.2.0
Development

Successfully merging this pull request may close these issues.

Enabling/disabling a Timepicker control is broken
4 participants
@FrancescoBorzi @ludmilanesvitiy @valorkin @Domainv