Skip to content

Conversation

@jpuri
Copy link
Contributor

@jpuri jpuri commented Dec 15, 2014

The PR will fix the following 2 major issue in move column functionality:

  1. Move column is broken if grid has pinned columns moveColumn service works incorrectly if user pins a column #2346.
  2. Move colimn broken in grid with huge horizontal scroll Move columns not working well on touch devices #2303.

@c0bra I have commented line no. 148 in ui-grid-header-cell.js, please review.

@jpuri
Copy link
Contributor Author

jpuri commented Dec 15, 2014

Hey @c0bra .

I commented this line s it was breaking move column functionality. Mouse click event for header cell was not reaching the move column handler. This line was added by you few days back.

@c0bra
Copy link
Contributor

c0bra commented Dec 15, 2014

I think after some additional pushes my comment was removed because the line(s) changed. Sorry for any mistake/confusion/breakage on my part!

I'm restarting the Travis job and it should run here shortly.

@jpuri
Copy link
Contributor Author

jpuri commented Dec 16, 2014

Hi @c0bra ,

That was a different PR where you has made a comment and I also replied @PaulL1 has merged that. In this PR I was mentioning your change in file ui-grid-header-cell.js, which I have commented as it was breaking move column functionality.

@jpuri jpuri force-pushed the master branch 4 times, most recently from 093751a to c092b7c Compare December 17, 2014 12:04
@jpuri
Copy link
Contributor Author

jpuri commented Dec 17, 2014

Just did some changes in PR to fix test cases that were breaking.

PaulL1 added a commit that referenced this pull request Dec 19, 2014
@PaulL1 PaulL1 merged commit 079f358 into angular-ui:master Dec 19, 2014
@c0bra c0bra removed the unknown label Dec 19, 2014
@PaulL1
Copy link
Contributor

PaulL1 commented Dec 19, 2014

Thanks again @jpuri for all your work.

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.

3 participants