Skip to content

fix(bug): visualization not being rendered correctly in PDF delivery schedule fix #71

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 34 additions & 30 deletions src/report_table.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const loadStylesheet = function (link) {
document.getElementsByTagName('head')[0].appendChild(linkElement);
};

const buildReportTable = function (
const buildReportTable = async function (
config,
dataTable,
updateColumnOrder,
Expand All @@ -42,20 +42,19 @@ const buildReportTable = function (
const chartCentreX = bounds.x + bounds.width / 2;
const chartCentreY = bounds.y + bounds.height / 2;

removeStyles().then(() => {
if (
typeof config.customTheme !== 'undefined' &&
config.customTheme &&
config.theme === 'custom'
) {
loadStylesheet(config.customTheme);
} else if (typeof themes[config.theme] !== 'undefined') {
themes[config.theme].use();
}
if (typeof themes[config.layout] !== 'undefined') {
themes[config.layout].use();
}
});
await removeStyles();
if (
typeof config.customTheme !== 'undefined' &&
config.customTheme &&
config.theme === 'custom'
) {
loadStylesheet(config.customTheme);
} else if (typeof themes[config.theme] !== 'undefined') {
themes[config.theme].use();
}
if (typeof themes[config.layout] !== 'undefined') {
themes[config.layout].use();
}

// Sort group based on sort order from looker
const sortByColumnSeries = function (group) {
Expand Down Expand Up @@ -228,17 +227,17 @@ const buildReportTable = function (
.style('font-size', config.headerFontSize + 'px')
.attr('draggable', true)
.call(drag)
.on('mouseover', function(cell) {
.on('mouseover', function (cell) {
d3.select('#tooltip')
.style('left', d3.event.x + 'px')
.style('top', d3.event.y + 'px')
.html(cell.label);
.style('left', d3.event.x + 'px')
.style('top', d3.event.y + 'px')
.html(cell.label);
d3.select('#tooltip').classed('hidden', false);
return dropTarget = cell
return (dropTarget = cell);
})
.on('mouseout', function(cell) {
.on('mouseout', function (cell) {
d3.select('#tooltip').classed('hidden', true);
return dropTarget = null
return (dropTarget = null);
});

var table_rows = table
Expand Down Expand Up @@ -527,7 +526,7 @@ const buildReportTable = function (
);
};

renderTable().then(() => {
await renderTable().then(() => {
document.getElementById('reportTable').classList.add('reveal');
if (config.customTheme === 'animate') {
document.getElementById('visSvg').classList.remove('hidden');
Expand Down Expand Up @@ -639,12 +638,17 @@ looker.plugins.visualizations.add({
// console.log(config)
var dataTable = new VisPluginTableModel(data, queryResponse, config);
this.trigger('registerOptions', dataTable.getConfigOptions());
buildReportTable(config, dataTable, updateColumnOrder, element);

// DEBUG OUTPUT AND DONE
// console.log('dataTable', dataTable)
// console.log('container', document.getElementById('visContainer').parentNode)

done();
buildReportTable(config, dataTable, updateColumnOrder, element)
.then(() => {
// DEBUG OUTPUT AND DONE
// console.log('dataTable', dataTable)
// console.log('container', document.getElementById('visContainer').parentNode)
})
.catch(error => {
console.error(error);
})
Comment on lines +647 to +649

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

It's good that you're catching errors from buildReportTable. However, just logging to the console might not be visible to end-users. It would be more robust to use this.addError to display a user-friendly error in the visualization, similar to how other errors are handled in this component.

      .catch(error => {
        console.error(error);
        this.addError({
          title: 'Error building report table',
          message: error.message,
        });
      })

.finally(() => {
done();
});
},
});