Skip to content

Commit 623b24d

Browse files
[O2B-416] View attachement of log-details entries in browser dialog (#1301)
* [O2B-416] Make attachments clickable * [O2B-416] Add conditional style/popover * [O2B-416] Show image preview onclick * [O2B-416] Show images on hover and add other file types * [O2B-416] Add JSON file type * [O2B-416] Add preview style and change popover config * [O2B-416] Add text file preview * [O2B-416] Change onCreate dom access to using the LogCreationModel * [O2B-416] Add CSV table style * [O2B-416] Add shellscript file type (.sh files) * [O2B-416] Fix single file case * [O2B-416] Add code documentation and fix lint errors * [O2B-416] Change tests for the new feature * [O2B-416] Change filePreview functionality to a separate directory * [O2B-416] Add file preview to the log-detail page * [O2B-416] Fixed blank line issue * [O2B-416] Make log-detail files downloadable * [O2B-416] Add preview seperator * [O2B-416] Move js style to css style * [O2B-416] Fix linter errors * [O2B-416] Fix tests * [O2B-416] Move js style to css style * [O2B-416] Fix linter error * [O2B-416] Apply pr feedback and move handling the file types to the model (list of methods for handling each file preview) * [O2B-416] Add RemoteData to the file preview model * [O2B-416] Fix new log file attachments not showing (RemoteData.notAsked always true) * [O2B-416] Fix eslint errors * [O2B-416] Fix changed code for passing all tests * Minor code cleaning * Improve overall structure * Revert change * Fix test * Fix code review * Remove useless authentication token * Fix spinner (again) * Fix preview size --------- Co-authored-by: martinboulais <31805063+martinboulais@users.noreply.github.com>
1 parent e1c0eaf commit 623b24d

14 files changed

Lines changed: 569 additions & 67 deletions

File tree

lib/public/app.css

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,8 @@ html, body {
7272
.w-90 { width: 90%; }
7373
.w-95 { width: 95%; }
7474
.w-100 { width: 100%; }
75+
.mw-100 { max-width: 100%; }
76+
.mh-100 { max-height: 100%; }
7577
.w-30rem { width: 30rem; }
7678
.w-inherit {
7779
width: inherit !important;

lib/public/components/Log/log.js

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,14 @@ import { logReplyButton } from './logReplyButton.js';
2222
import { logText } from './logText.js';
2323
import { primaryButton } from '../common/primaryButton.js';
2424
import { logDetailsButton } from './logDetailsButton.js';
25+
import { attachmentPreviewComponent } from '../../views/Logs/FilePreview/attachmentPreviewComponent.js';
2526
import { tagBadge } from '../tag/tagBadge.js';
2627

2728
/**
2829
* Provides formatting for each log field to display that field on the webpage
29-
* @param {string} authenticationToken token to authenticate api requests. Used for attachments
3030
* @return {Object} an object determining how each log field wil lbe displayed on the page
3131
*/
32-
const logFields = (authenticationToken) => ({
32+
const logFields = () => ({
3333
author: {
3434
name: 'Source',
3535
format: (author) => author.name,
@@ -69,15 +69,10 @@ const logFields = (authenticationToken) => ({
6969
attachments: {
7070
name: 'Attachments',
7171
visible: true,
72-
format: (attachments) => detailsFrontLinks(
73-
attachments,
74-
({ fileName, originalName, id }) => ({
75-
content: originalName,
76-
href: `/api/attachments/${id}?token=${authenticationToken}`,
77-
attributes: { download: fileName, class: 'break-all' },
78-
}),
79-
{ bypassRouter: true },
80-
),
72+
format: (attachments) => h('span.flex-row.flex-wrap.gc1', attachments.flatMap((attachment) => [
73+
attachmentPreviewComponent(attachment),
74+
h('span.mr2', ','),
75+
]).slice(0, -1)),
8176
},
8277
});
8378

@@ -93,7 +88,8 @@ const expandedLogFields = ['runs', 'environments', 'lhcFills', 'attachments'];
9388
* @param {function} onCollapse function called when the log should be collapsed
9489
* @param {function} onExpand function called when the log should be expanded
9590
* @param {Object} options other options
96-
* @param {string} [options.authenticationToken] token to authenticate the session with the api
91+
* @param {boolean} [options.replyButton] if true, reply button will be displayed
92+
* @param {boolean} [options.goToDetailsButton] if true, button to go to details will be displayed
9793
* @param {Object} [options.logMarkdownDisplayOpts] markdown display opts @see logText opts argument
9894
* @param {Partial<MarkdownBoxSize>} [options.logMarkdownDisplayOpts.boxSize] size of markdown display
9995
* @return {Component} the log's display component
@@ -108,15 +104,15 @@ export const logComponent = (
108104
options = {},
109105
) => {
110106
const { title = '', id = '' } = log;
111-
const fieldFormatting = logFields(options?.authenticationToken);
107+
const fieldFormatting = logFields();
112108
const logTitle = showLogTitle ? h(`h4#log-${id}-title`, title) : '';
113109
const displayedLogFields = isCollapsed ? [] : expandedLogFields;
114110
const logViewButton = isCollapsed
115111
? primaryButton('Show', onExpand, `show-collapse-${id}`)
116112
: primaryButton('Collapse', onCollapse, `collapse-${id}`);
117113

118114
return h(
119-
`#log-${id}.br2.m1.p3.shadow-level1.flex-column.g3${highlight ? '.b1.b-primary' : ''}`,
115+
`#log-${id}.br2.m1.p3.shadow-level1.flex-column.g1${highlight ? '.b1.b-primary' : ''}`,
120116
h('div.flex-row.justify-between.', [
121117
h(`div.flex-column.g1.log-details-${isCollapsed ? 'collapsed' : 'expanded'}`, [
122118
logTitle,
@@ -125,16 +121,16 @@ export const logComponent = (
125121
h('em', `${fieldFormatting.author.format(log.author)} (${fieldFormatting.createdAt.format(log.createdAt)})`),
126122
),
127123
fieldFormatting.tags.format(log.tags),
128-
logDetails(log, displayedLogFields, fieldFormatting),
129124
]),
130125
h('div.flex-row.flex-shrink-0.flex-wrap.items-start.g1', [
131126
logLinkButton(log),
132-
(options.replayButton ?? true) && logReplyButton(log),
133-
(options.goToDetailsButton ?? false) && logDetailsButton(log),
127+
options.replyButton && logReplyButton(log),
128+
options.goToDetailsButton && logDetailsButton(log),
134129
logViewButton,
135130
]),
136131
]),
137-
h('.bt1.b-gray-light'),
132+
logDetails(log, displayedLogFields, fieldFormatting),
133+
h('.bt1.b-gray-light.pv2/'),
138134
logText(log, isCollapsed, options?.logMarkdownDisplayOpts),
139135
);
140136
};

lib/public/components/Log/logDetails.js

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,7 @@ import { h } from '/js/src/index.js';
2121
* @param {Object} fieldFormats an object containing a format function for each field
2222
* @return {Component} the log field displayed
2323
*/
24-
export const logDetails = (log, fields, fieldFormats) => fields.map((key) =>
25-
h(`div#log-${log.id}-${key}.flex-row`, [
26-
h('div.mr2', `${fieldFormats[key].name}:`),
27-
fieldFormats[key].format(log[key]),
28-
]));
24+
export const logDetails = (log, fields, fieldFormats) => fields.map((key) => h(`div#log-${log.id}-${key}.flex-row`, [
25+
h('div.mr2', `${fieldFormats[key].name}:`),
26+
fieldFormats[key].format(log[key]),
27+
]));

lib/public/components/common/errorAlert.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import { h } from '/js/src/index.js';
2323
* Generic error alert.
2424
*
2525
* @param {ApiError[]|ApiError} errors The JSON:API conform error object.
26-
* @returns {vnode[]} The list of vnodes representing the alerts.
26+
* @returns {Component} the error alerts component
2727
*/
2828
const errorAlert = (errors) => {
2929
if (!Array.isArray(errors)) {

lib/public/components/common/navigation/frontLinks.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ export const frontLinks = (items, configurationExtractor, configuration) => {
7474

7575
return [link, separator];
7676
});
77+
7778
// Remove separator from last link
7879
const lastLinkWithSeparator = linksWithSeparator[linksWithSeparator.length - 1];
7980
lastLinkWithSeparator.pop();

lib/public/components/common/popover/PopoverEngine.js

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,12 @@ class PopoverEngine {
192192
this._popover.style.removeProperty('width');
193193
this._popover.style.removeProperty('top');
194194
this._popover.style.removeProperty('left');
195+
if (this._setChildrensize) {
196+
for (const child of this._popover.children) {
197+
child.style.removeProperty('height');
198+
child.style.removeProperty('width');
199+
}
200+
}
195201
this._popoverBoundingBox = this._popover.getBoundingClientRect();
196202
}
197203

@@ -227,8 +233,8 @@ class PopoverEngine {
227233

228234
this._mainAxisProjector.setSize(this._popover.style, `${mainAxisSize}px`);
229235
if (this._setChildrensize) {
230-
for (const children of this._popover.children) {
231-
this._mainAxisProjector.setSize(children.style, `${mainAxisSize}px`);
236+
for (const child of this._popover.children) {
237+
this._mainAxisProjector.setSize(child.style, `${mainAxisSize}px`);
232238
}
233239
}
234240

@@ -245,8 +251,8 @@ class PopoverEngine {
245251
if (this._popoverSizeCrossAxis > this._availableSpaceInCrossAxis) {
246252
this._crossAxisProjector.setSize(this._popover.style, `${this._availableSpaceInCrossAxis}px`);
247253
if (this._setChildrensize) {
248-
for (const children of this._popover.children) {
249-
this._crossAxisProjector.setSize(children.style, `${this._availableSpaceInCrossAxis}px`);
254+
for (const child of this._popover.children) {
255+
this._crossAxisProjector.setSize(child.style, `${this._availableSpaceInCrossAxis}px`);
250256
}
251257
}
252258

lib/public/views/Logs/Create/LogCreationModel.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ export class LogCreationModel extends Observable {
4242
this._creationTagsPickerModel = new TagPickerModel();
4343
this._creationTagsPickerModel.bubbleTo(this);
4444
this._creationTagsPickerModel.visualChange$.bubbleTo(this);
45+
4546
this._parentLogId = null;
4647

4748
this._isParentLogCollapsed = true;

lib/public/views/Logs/Create/logCreationComponent.js

Lines changed: 67 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,17 @@ import { tagPicker } from '../../../components/tag/tagPicker.js';
1818
import { markdownInput } from '../../../components/common/markdown/markdown.js';
1919
import spinner from '../../../components/common/spinner.js';
2020
import { logComponent } from '../../../components/Log/log.js';
21+
import { filePreviewPopoverLink } from '../FilePreview/attachmentPreviewComponent.js';
22+
import { popover } from '../../../components/common/popover/popover.js';
23+
import { PopoverTriggerPreConfiguration } from '../../../components/common/popover/popoverPreConfigurations.js';
24+
import { PopoverAnchors } from '../../../components/common/popover/PopoverEngine.js';
25+
import { filePreviewComponent } from '../FilePreview/filePreviewComponent.js';
2126

2227
/**
2328
* A function to construct the create log screen
2429
*
2530
* @param {LogCreationModel} creationModel - Log creation model
2631
* @param {Object} options component creation options
27-
* @param {string} [options.authenticationToken] token to authenticate the session with the api
2832
* @param {Object} [options.parentLogMarkdownDisplayOptions] markdown display options @see logText options argument
2933
* @return {vnode} the log creation component
3034
*/
@@ -47,7 +51,6 @@ export const logCreationComponent = (creationModel, options) => {
4751
* @param {LogCreationModel} creationModel the creation model
4852
* @param {Log|null} parentLog - The parent of the log being created
4953
* @param {Object} options component creation options
50-
* @param {string} [options.authenticationToken] token to authenticate the session with the api
5154
* @param {Object} [options.parentLogMarkdownDisplayOptions] markdown display options @see logText options argument
5255
* @return {vnode} the form component
5356
*/
@@ -75,10 +78,10 @@ const logCreationForm = (creationModel, parentLog, options) => [
7578
creationModel.isParentLogCollapsed,
7679
() => creationModel.collapseParentLog(),
7780
() => creationModel.showFullParentLog(),
78-
{ ...options,
81+
{
82+
...options,
7983
logMarkdownDisplayOpts: options.parentLogMarkdownDisplayOptions,
8084
goToDetailsButton: true,
81-
replayButton: false,
8285
},
8386
),
8487

@@ -116,7 +119,9 @@ const labeledInput = (labelAttributes, labelText, content, containerAttributes)
116119
labelAttributes,
117120
labelText,
118121
),
119-
content,
122+
...Array.isArray(content)
123+
? content
124+
: [content],
120125
],
121126
);
122127
};
@@ -234,28 +239,74 @@ const environmentsPanel = (logCreationModel) => labeledInput(
234239
* @return {vnode} - panel allowing the user to attach files
235240
*/
236241
const attachmentsPanel = (logCreationModel) => {
237-
const { attachments } = logCreationModel;
242+
const { attachments: attachmentsFileList } = logCreationModel;
243+
244+
/**
245+
* @type {File[]}
246+
*/
247+
const attachments = [...attachmentsFileList];
248+
249+
let fileText;
250+
switch (attachments.length) {
251+
case 0:
252+
fileText = 'No files chosen';
253+
break;
254+
case 1:
255+
fileText = `${attachments.length} file chosen`;
256+
break;
257+
default:
258+
fileText = `${attachments.length} files chosen`;
259+
break;
260+
}
238261

239262
return labeledInput(
240263
{ for: 'attachments' },
241264
'File attachments',
242-
h('.flex-column.p2.g2', [
243-
h('.flex-row.justify-between.items-center', [
244-
h('input#attachments.w-33', {
265+
[
266+
h('.flex-row.justify-between.items-center.p2', [
267+
h(
268+
'label.flex-row.g2.items-center',
269+
{ for: 'attachments' },
270+
[
271+
h('.btn', 'Choose files'),
272+
h('', fileText),
273+
],
274+
),
275+
h('input#attachments.w-33.d-none', {
245276
type: 'file',
246277
multiple: true,
247278
onchange: (e) => {
248279
logCreationModel.attachments = e.target.files;
249280
},
250281
}),
251-
attachments.length > 0 && h('button#clearAttachments.btn.btn-danger.ml3', {
252-
onclick: () => logCreationModel.clearAttachments(),
253-
}, 'Clear'),
282+
attachments.length > 0 && h(
283+
'button#clearAttachments.btn.btn-danger.ml3',
284+
{ onclick: () => logCreationModel.clearAttachments() },
285+
'Clear',
286+
),
254287
]),
255-
attachments.length > 1 && h('#attachmentNames', {
256-
style: 'min-height: 1.5em;',
257-
}, [...attachments].map((attachment) => attachment.name).join(', ')),
258-
]),
288+
h('#attachments-list.flex-row.flex-wrap.p2', attachments.flatMap((attachment) => {
289+
const preview = filePreviewComponent(attachment, attachment.type, attachment.name);
290+
return [
291+
preview
292+
? popover(
293+
filePreviewPopoverLink(attachment.name),
294+
h(
295+
'.p2.flex-row.items-center',
296+
{ key: window.btoa(`${attachment.name}-${attachment.size}-${attachment.lastModified}`) },
297+
preview,
298+
),
299+
{
300+
...PopoverTriggerPreConfiguration.click,
301+
anchor: PopoverAnchors.TOP_MIDDLE,
302+
setChildrenSize: true,
303+
},
304+
)
305+
: h('', attachment.name),
306+
h('span.mr2', ','),
307+
];
308+
}).slice(0, -1)),
309+
],
259310
);
260311
};
261312

lib/public/views/Logs/Details/logTreeViewComponent.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,9 @@ import { frontLink } from '../../../components/common/navigation/frontLink.js';
2121
/**
2222
* A collection of details relating to a log
2323
* @param {LogTreeViewModel} treeViewModel model of the tree view
24-
* @param {string} authenticationToken the token to use to display attachments
2524
* @return {Component} Return the view of the table with the filtering options
2625
*/
27-
export const logTreeViewComponent = (treeViewModel, authenticationToken) => {
26+
export const logTreeViewComponent = (treeViewModel) => {
2827
const { detailedPostsIds, selectedLogId, logTreeLeafs: logTreeLeafsRemoteData } = treeViewModel;
2928
return logTreeLeafsRemoteData.match({
3029
NotAsked: () => null,
@@ -43,11 +42,13 @@ export const logTreeViewComponent = (treeViewModel, authenticationToken) => {
4342
logComponent(
4443
log,
4544
log.id === selectedLogId,
46-
!(log.title === parentLogTitle),
45+
log.title !== parentLogTitle,
4746
isCollapsed,
4847
() => treeViewModel.collapseLog(log.id),
4948
() => treeViewModel.expandLog(log.id),
50-
{ authenticationToken },
49+
{
50+
replyButton: true,
51+
},
5152
),
5253
);
5354

0 commit comments

Comments
 (0)