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.

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

TypeScript Reporting Layouts #22454

Merged
merged 13 commits into from
Sep 4, 2018
8 changes: 4 additions & 4 deletions src/server/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
* you may not use this file except in compliance with the Elastic License.
*/
export interface KbnServer {
config: () => configObject;
}
config: () => configObject;
}

export interface configObject {
Copy link
Contributor

Choose a reason for hiding this comment

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

configObject => ConfigObject

get: (path: string) => any;
}
get: (path: string) => any;
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { pdf } from './pdf';
import { groupBy } from 'lodash';
import { oncePerServer } from '../../../../server/lib/once_per_server';
import { screenshotsObservableFactory } from './screenshots';
import { createlayout } from './layouts/layout_factory';
import { createLayout } from './layouts/layout_factory';

const getTimeRange = (urlScreenshots) => {
const grouped = groupBy(urlScreenshots.map(u => u.timeRange));
Expand Down Expand Up @@ -68,7 +68,7 @@ function generatePdfObservableFn(server) {

return function generatePdfObservable(title, urls, browserTimezone, headers, layoutParams, logo) {

const layout = createlayout(server, layoutParams);
const layout = createLayout(server, layoutParams);

const screenshots$ = urlScreenshotsObservable(urls, headers, layout);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
import { Size } from '../../../../../types';

export interface CaptureConfig {
zoom: number;
viewport: Size;
zoom: number;
viewport: Size;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

-2 spaces for the }


export interface ViewZoomWidthHeight {
zoom: number;
width: number;
height: number;
}
zoom: number;
width: number;
height: number;
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export abstract class Layout {

public abstract getPdfPageOrientation(): string | undefined;

public abstract getPdfPageSize(pageSizeParamsIn: PageSizeParams): string | Size;
public abstract getPdfPageSize(pageSizeParams: PageSizeParams): string | Size;

public abstract getViewport(itemsCount: number): ViewZoomWidthHeight;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,22 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { KbnServer } from '../../../../../../../../src/server/index';
import { Size } from '../../../../../types';
import { LayoutTypes } from '../../../common/constants';
import { Layout } from './layout';
import { PreserveLayout } from './preserve_layout';
import { PrintLayout } from './print_layout';

// you'll notice that we aren't passing the zoom at this time, while it'd be possible to use
// window.pixelDensity to figure out what the current user is seeing, if they're going to send the
// PDF to someone else, I can see there being benefit to using a higher pixel density, so we're
// going to leave this hard-coded for the time being

interface LayoutParams {
id: string;
dimensions: {
height: number;
width: number;
};
dimensions: Size;
}

export function createlayout(server: KbnServer, LayoutParamsin: LayoutParams): Layout {
if (LayoutParamsin && LayoutParamsin.id === LayoutTypes.PRESERVE_LAYOUT) {
return new PreserveLayout(LayoutParamsin.id, LayoutParamsin.dimensions);
export function createLayout(server: KbnServer, layoutParams: LayoutParams): Layout {
Copy link
Member

Choose a reason for hiding this comment

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

let's call this file create_layout.ts to name it after what its primary export is.

if (layoutParams && layoutParams.id === LayoutTypes.PRESERVE_LAYOUT) {
return new PreserveLayout(layoutParams.id, layoutParams.dimensions);
}

// this is the default because some jobs won't have anything specified
return new PrintLayout(server, LayoutParamsin.id);
return new PrintLayout(server, layoutParams.id);
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { Layout, PageSizeParams } from './layout';
const ZOOM: number = 2;
Copy link
Member

Choose a reason for hiding this comment

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

Zoom used to be a param that could be passed in from a caller, but 2 was the default. This makes it completely hardcoded.

Also, you've removed the explanatory comments: https://github.com/elastic/kibana/blob/master/x-pack/plugins/reporting/export_types/printable_pdf/server/lib/layouts/preserve_layout.js#L9-L12

Copy link
Contributor

Choose a reason for hiding this comment

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

Intentional. This setting was never exposed to the user as configurable - #20091

I don't think we ever will expose it. The only reason we use it is to make sure the resolution is a little bigger on the screenshots.

From Brandon in that above issue:

The zoom is an undocumented setting that I intended to remove, but never got around to it.


export class PreserveLayout extends Layout {
public selectors = {
public readonly selectors = {
screenshot: '[data-shared-items-container]',
renderComplete: '[data-shared-item]',
itemsCountAttribute: 'data-shared-items-count',
Expand Down Expand Up @@ -66,16 +66,16 @@ export class PreserveLayout extends Layout {
return undefined;
}

public getPdfPageSize(pagesizeparams: PageSizeParams) {
public getPdfPageSize(pageSizeParams: PageSizeParams) {
return {
height:
this.height +
pagesizeparams.pageMarginTop +
pagesizeparams.pageMarginBottom +
pagesizeparams.tableBorderWidth * 2 +
pagesizeparams.headingHeight +
pagesizeparams.subheadingHeight,
width: this.width + pagesizeparams.pageMarginWidth * 2 + pagesizeparams.tableBorderWidth * 2,
pageSizeParams.pageMarginTop +
pageSizeParams.pageMarginBottom +
pageSizeParams.tableBorderWidth * 2 +
pageSizeParams.headingHeight +
pageSizeParams.subheadingHeight,
width: this.width + pageSizeParams.pageMarginWidth * 2 + pageSizeParams.tableBorderWidth * 2,
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { Layout } from './layout';
type EvalArgs = any[];

interface EvaluateOptions {
// Fn is a function in string form to avoid tslint from auto formatting it into a version not
// 'fn' is a function in string form to avoid tslint from auto formatting it into a version not
// underfood by transform_fn safeWrap.
fn: ((evalArgs: EvalArgs) => any) | string;
args: EvalArgs; // Arguments to be passed into the function defined by fn.
Expand All @@ -22,8 +22,6 @@ interface BrowserClient {
evaluate: (evaluateOptions: EvaluateOptions) => void;
}

const groupCount: number = 2;

export class PrintLayout extends Layout {
public selectors = {
screenshot: '[data-shared-item]',
Expand All @@ -33,6 +31,8 @@ export class PrintLayout extends Layout {
timefilterToAttribute: 'data-shared-timefilter-to',
};

public readonly groupCount = 2;

private captureConfig: CaptureConfig;

constructor(server: KbnServer, id: string) {
Expand Down
14 changes: 7 additions & 7 deletions x-pack/plugins/reporting/types.d.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
export interface Size {
width: number;
height: number;
}
width: number;
height: number;
}

export interface Logger {
debug: (message: string) => void;
error: (message: string) => void;
warning: (message: string) => void;
}
debug: (message: string) => void;
error: (message: string) => void;
warning: (message: string) => void;
}