Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Avoid interfering with layout of tables with absolutely positioned content #641

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Avoid interfering with layout of tables with absolutely positioned content #641

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bba4588
Avoid interfering with layout of tables with absolutely positioned co…
Mar 10, 2024
032d8bf
Replace reduce() with some()
Mar 10, 2024
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
25 changes: 20 additions & 5 deletions iXBRLViewerPlugin/viewer/src/js/tableExport.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,26 @@ export class TableExport {
$('table', iframe).each(function () {
const table = $(this);
if (table.find(".ixbrl-element").length > 0) {
table.css("position", "relative");
const exporter = new TableExport(table, reportSet);
$('<div class="ixbrl-table-handle"><span>Export table</span></div>')
.appendTo(table)
.click(() => exporter.exportTable());
const containsAbsolute = Array.from(this.querySelectorAll("*"))
.reduce(
austinmatherne-wk marked this conversation as resolved.
Show resolved Hide resolved
(res, elt) => (res || getComputedStyle(elt).getPropertyValue('position') === 'absolute'),
false
);

// Don't add handles if the table contains absolutely
// positioned elements, as doing so will probably interfere
// with layout.
//
// Where a table has absolutely positioned content, it's quite
// likely that the table tag itself is positioned in a random
// place.
Copy link
Contributor

Choose a reason for hiding this comment

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

I worry this will break/remove the handle from Workiva-generated documents, which use position:absolute on all <div> within a <table> but I believe position the <table> in a specific/non-random place (test).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it looks like the table has been carefully constructed to ensure that it's position and size matches the rendered content, so in this case if we were to put the handle at the top left of the table, it would work.

We could potentially detect that the table has non-zero size, and show the handle in that case, but we still have the issue that adding position: relative may interfere with the layout. In this example, the cell contents are positioned relative to an enclosing <div> which happens to have the same position as the <table> element, so adding position: relative has no impact. But, an innocuous change like adding a top margin to the table, means that adding position: relative breaks the layout because the cell contents are positioned relative to a different location.

We could find the minimum top and left positions of all the absolutely positioned children, and put the export handle at that location, although we'd need to take care that the children are actually position relative to the table or some parent.

Elsewhere, we've discussed replacing all highlighting / borders / annotation with constructs that are positioned relative to to the top of the document, but would be a significant change, and would require recalculating positions if the window gets resized.

Copy link
Contributor

Choose a reason for hiding this comment

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

I saw a link to this blog post over the weekend, which seemed promising:

Once you can use this freely, you’ll have to care less about exact DOM positioning of elements (aside from accessibility concerns). The way it is now, the element you want to position relative to another has to be a child element and for there to be a positioning context to work within. This can dictate where elements go in the DOM, whether or not that makes sense.

Looks like Firefox intends to prototype, which is promising.

if (!containsAbsolute) {
table.css("position", "relative");
const exporter = new TableExport(table, reportSet);
$('<div class="ixbrl-table-handle"><span>Export table</span></div>')
.appendTo(table)
.click(() => exporter.exportTable());
}
}
});
}
Expand Down
7 changes: 6 additions & 1 deletion samples/src/testdoc/testdoc.ixtmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,13 +164,18 @@ Profit Loss double tagged: {{ monetary eg:ProfitLoss[] }}{{ monetary eg:ProfitLo
{{ period cur3mo "2018-09-01" "2018-12-31" }}
{{ period prev3mo "2017-09-01" "2017-12-31" }}

<p>
This table contains some absolutely position content, so should not
have a table export handle.
</p>

<table class="accounts">
{{ column-aspects static [period=cur] [period=prev] [period=cur3mo] [period=prev3mo] }}
{{ column-styles "" "figure" "figure" "figure" "figure" }}
<thead>
<tr>
<th></th>
<th>2018</th>
<th style="position: relative"><div style="position: absolute">2018</div></th>
<th>2017</th>
<th>3 months to <br></br>31 Dec 2018</th>
<th>3 months to <br></br>31 Dec 2017</th>
Expand Down
Loading