-
Notifications
You must be signed in to change notification settings - Fork 42
Improvements to Null values handling #409
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
base: main
Are you sure you want to change the base?
Conversation
worksofliam
left a comment
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 made a few more changes:
|
|
Note: I just met @dferrand who asked for the status of the PR. This is a bump to not forget it 😅 |
|
@dferrand can you resolve that small remaining conflict? Then I'll give it a go. Thanks! |
sebjulliand
left a comment
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.
It's a great change, thanks @dferrand .
There are two little quirks though:
- Label and Name are inverted (see my comment)
- When displaying the joblog, cell content is not centered in the middle as it did before. Can this be fixed?
Fixed these two points and resolve the remaining conflict and we'll be good to go !
| case 'Name': | ||
| cell.innerText = column.label; | ||
| break; | ||
| case 'Both': | ||
| cell.innerHTML = escapeHTML(column.name)+'<br>'+escapeHTML(column.label); | ||
| break; | ||
| default: | ||
| cell.innerText = column.name; |
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.
name and label are inverted here/
| case 'Name': | |
| cell.innerText = column.label; | |
| break; | |
| case 'Both': | |
| cell.innerHTML = escapeHTML(column.name)+'<br>'+escapeHTML(column.label); | |
| break; | |
| default: | |
| cell.innerText = column.name; | |
| case 'Name': | |
| cell.innerText = column.name; | |
| break; | |
| case 'Both': | |
| cell.innerHTML = escapeHTML(column.name)+'<br>'+escapeHTML(column.label); | |
| break; | |
| default: | |
| cell.innerText = column.label; |




Multiple changes to null value handling: