Skip to content

[BUGFIX] minColumnWidth fix #753

Merged
bakesteve merged 3 commits intoComcast:masterfrom
martinnov92:master
Jun 30, 2017
Merged

[BUGFIX] minColumnWidth fix #753
bakesteve merged 3 commits intoComcast:masterfrom
martinnov92:master

Conversation

@martinnov92
Copy link
Copy Markdown
Contributor

Description

Hello everyone,

in this pull request I tried to fix this weird behavior of not respecting minColumnWidth when resizing the window or grid.
Current situation is when you are resizing the window, all the columns are calculating its width and don't respect their set minColumnWidth which may cause that they become really small (here is the video), but after this fix, once their width become less then minColumnWidth, they keep the minColumnWidth - video

Hopefully I am not the only one who would appreciated this fix 😄 .

Thank you

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x")

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior? (You can also link to an open issue here)

Current situation is when you are resizing the window, all the columns are calculating its width and don't respect their set minColumnWidth which may cause that they become really small (here is the video)

What is the new behavior?

After this fix, once their width become less then minColumnWidth, they keep the minColumnWidth - video

Does this PR introduce a breaking change? (check one with "x")

[ ] Yes
[x] No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

@coveralls
Copy link
Copy Markdown

Coverage Status

Changes Unknown when pulling 8acbd45 on martinnov92:master into ** on adazzle:master**.

@martinnov92 martinnov92 changed the title minColumnWidth fix [BUGFIX] minColumnWidth fix Apr 20, 2017
@martinnov92
Copy link
Copy Markdown
Contributor Author

Hello @jpdriver and @malonecj , do you mind making a code review? thank you

@jpdriver
Copy link
Copy Markdown
Contributor

this looks good to me.. @malonecj please can you merge?

@jpdriver
Copy link
Copy Markdown
Contributor

ps thank you so much for the videos @martinnov92 -- if only all PRs had them! 😛

@martinnov92
Copy link
Copy Markdown
Contributor Author

@jpdriver Thank you too :)

@martinnov92
Copy link
Copy Markdown
Contributor Author

Hello @malonecj and @jpdriver , do you think this will get merge anytime soon? thank you

@jpdriver
Copy link
Copy Markdown
Contributor

would have already merged this if i could @martinnov92 ! unfortunately i can't help -- you'll just have to wait until Conor has time to look at it.

@martinnov92
Copy link
Copy Markdown
Contributor Author

@jpdriver Ok, thank you :)

@mitchelldemler
Copy link
Copy Markdown

Bump. This is pretty crucial for a responsive table.

@bakesteve bakesteve merged commit db9c920 into Comcast:master Jun 30, 2017
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.

5 participants