db_central_columns: new column collapsing issue #12939

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@poush
Contributor
poush commented Jan 31, 2017 edited

Fixes: #12904
Signed-off-by: Piyush Agrawal poush12@gmail.com

Before submitting pull request, please check that every commit:

  • Has proper Signed-Off-By
  • Has commit message which describes it
  • Is needed on it's own, if you have just minor fixes to previous commits, you can squash them
  • Any new functionality is covered by tests
@codecov-io
codecov-io commented Jan 31, 2017 edited

Codecov Report

❗️ No coverage uploaded for pull request base (master@8a6d7de). Click here to learn what that means.

@@            Coverage Diff            @@
##             master   #12939   +/-   ##
=========================================
  Coverage          ?   54.23%           
=========================================
  Files             ?      466           
  Lines             ?    69589           
  Branches          ?        0           
=========================================
  Hits              ?    37743           
  Misses            ?    31846           
  Partials          ?        0

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 8a6d7de...aa3814a. Read the comment docs.

@nijel
Member
nijel commented Feb 2, 2017 edited

When touching the code, can you please also remove the inline CSS? See #12262

@poush
Contributor
poush commented Feb 2, 2017

Done!

libraries/central_columns.lib.php
{
$addNewColumn = '<div id="add_col_div" class="topmargin"><a href="#">'
. '<span>+</span> ' . __('Add new column') . '</a>'
- . '<form id="add_new" style="min-width:100%;display:none" '
+ . '<form id="add_new" class="new_central_col '
+ . ($total_rows != 0 ? 'hidden"' : '"')
@nijel
nijel Feb 2, 2017 Member

Better to use existing class hide than introduce new one with same function.

@poush poush db_central_columns: new column collapsing issue
Signed-off-by: Piyush Agrawal <poush12@gmail.com>
aa3814a
@poush
Contributor
poush commented Feb 5, 2017

@nijel Actually I had "bootstrap" classes in my mind therefore searched for "hidden" class. Updated again!

@nijel nijel self-assigned this Feb 6, 2017
@nijel nijel added this to the 4.7.0 milestone Feb 6, 2017
@nijel nijel added a commit that referenced this pull request Feb 6, 2017
@nijel nijel Changelog entry for #12904 and #12939
Signed-off-by: Michal Čihař <michal@cihar.com>
80de044
@nijel
Member
nijel commented Feb 6, 2017

Thanks, merged as b575f7e (to include it in 4.7 maintenance branch).

@nijel nijel closed this Feb 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment