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.
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
Support notes field as a column field #690
Conversation
*/ | ||
function bugnote_get_all_visible_as_string( $p_bug_id, $p_user_bugnote_order, $p_user_bugnote_limit, $p_user_id = null ) { | ||
$t_notes = bugnote_get_all_visible_bugnotes( $p_bug_id, $p_user_bugnote_order, $p_user_bugnote_limit, $p_user_id ); | ||
$t_date_format = config_Get( 'normal_date_format' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lower case config_get
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Not sure how this happened :)
a782892
to
798409b
Compare
* @access public | ||
*/ | ||
function print_column_title_notes( $p_sort, $p_dir, $p_columns_target = COLUMNS_TARGET_VIEW_PAGE ) { | ||
echo '<th class="column-description">'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
column-notes?
798409b
to
ff8c3c9
Compare
@atrol Thanks for the review. I pushed an updated version with your feedback addressed. |
@@ -85,12 +85,13 @@ td.my-buglist-description { width: 100%; vertical-align: top; } | |||
/** | |||
* view_all_bug_page.php | |||
*/ | |||
#buglist td, #buglist th { text-align: center; } | |||
#buglist td, #buglist th { text-align: center; vertical-align: top; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the th
should be top-aligned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Depending on the number of notes associated to issues, including this column may have a non-negligible impact on performance. What happens if bugnote text already contains "\n" ? In other words, is it an issue that the column contents can't be broken down further after e.g. download to XL. |
The column styles were not applied to print page. We should either apply the styles to View Issues and Print Issues pages or have a set of styles for each.
- Added support for notes field in View Issues, Print Issues, CSV, and Excel. - Add vertical align top to the style for column values which is useful when the view includes a multi-line column. Fixes #20383
ff8c3c9
to
e5a99c0
Compare
@dregad The notes column is not really practical in the common usage scenario, i.e. View Issues page. It is mostly requested for csv/excel export where performance isn't really an issue. As for the new lines in notes, it is not really an issue. We have a line separator between notes and first line of each notes refers to author, timestamp, private vs. public, etc. It is mainly for human consumption re-parsing is do-able, but I don't see it as the main scenario. |
and Excel.
useful when the view includes a multi-line column.
Fixes #20383