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

Further Time Tacking Improvement #4121

Merged
merged 19 commits into from
Jul 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
6b681cd
duration of time tracking view is now in mins and secs
MichaelBuessemeyer Jun 3, 2019
019ec0a
Merge branch 'master' into improve-time-tracking
MichaelBuessemeyer Jun 13, 2019
f0839c1
refactored time tracking view and added spinning wheel when loading
MichaelBuessemeyer Jun 13, 2019
758e175
moved time tracking chart into an own file;
MichaelBuessemeyer Jun 13, 2019
74154d6
fixed flow errors
MichaelBuessemeyer Jun 13, 2019
664ee19
displaying chart now at full size always
MichaelBuessemeyer Jun 14, 2019
ebb811b
fixed tooltip positioning
MichaelBuessemeyer Jun 17, 2019
75d02aa
Merge branch 'master' of github.com:scalableminds/webknossos into imp…
MichaelBuessemeyer Jun 17, 2019
0725c02
Update CHANGELOG.md
MichaelBuessemeyer Jun 17, 2019
faf8961
added fixed height to chart again to avoid bugs;
MichaelBuessemeyer Jun 20, 2019
3a35be8
merged origin master into this branch
MichaelBuessemeyer Jun 20, 2019
cd65e8a
adding mispositioned tooltip at window borders fix
MichaelBuessemeyer Jun 20, 2019
c62574e
fixed the tooltip edgecase positioning bug
MichaelBuessemeyer Jun 20, 2019
f04726f
Merge branch 'master' of github.com:scalableminds/webknossos into imp…
MichaelBuessemeyer Jun 20, 2019
9010b48
Merge branch 'master' into improve-time-tracking
MichaelBuessemeyer Jun 21, 2019
117fb5a
Merge branch 'master' into improve-time-tracking
MichaelBuessemeyer Jun 27, 2019
d88505e
Merge branch 'master' into improve-time-tracking
MichaelBuessemeyer Jun 27, 2019
11a2535
Merge branch 'master' into improve-time-tracking
MichaelBuessemeyer Jul 2, 2019
bd85c7b
Merge branch 'master' into improve-time-tracking
MichaelBuessemeyer Jul 2, 2019
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,13 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.md).
- Added an two additional buttons to the dropdown menu of the tree hierarchy view. On Click, one collapses the other expands all subgroups. [#4143](https://github.com/scalableminds/webknossos/pull/4143)

### Changed
- The tooltip of the timeline chart in the Time Tracking view now displays the duration in minutes:seconds. [#4121](https://github.com/scalableminds/webknossos/pull/4121)
- Reactivated and renamed the "Quality" setting to "Hardware Utilization". Using a higher value will render data in higher quality, but puts more stress on the user's hardware and bandwidth. [#4142](https://github.com/scalableminds/webknossos/pull/4142)


### Fixed
- Fixed that team managers couldn't view time tracking details of other users anymore. [#4125](https://github.com/scalableminds/webknossos/pull/4125)
- Fixed the positioning of the tooltip of the timeline chart in the Time Tracking view. [#4121](https://github.com/scalableminds/webknossos/pull/4121)
- Fixed a rendering problem which caused a red viewport on some Windows machines. [#4133](https://github.com/scalableminds/webknossos/pull/4133)

### Removed
Expand Down
113 changes: 113 additions & 0 deletions frontend/javascripts/admin/time/time_line_chart_view.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
// @flow
import { Chart } from "react-google-charts";
import * as React from "react";
import { getWindowBounds } from "libs/utils";

export type ColumnDefinition = {
id?: string,
type: string,
role?: string,
p?: Object,
};

export type RowContent = [string, string, string, Date, Date];

export type DateRange = [moment$Moment, moment$Moment];

type Props = {
columns: Array<ColumnDefinition>,
rows: Array<RowContent>,
timeAxisFormat: string,
dateRange: DateRange,
};

export default class TimeTrackingChart extends React.PureComponent<Props> {
additionalCSS: ?HTMLStyleElement = null;
chartScrollElement: ?HTMLElement = null;

componentWillUnmount() {
if (this.chartScrollElement) {
this.chartScrollElement.removeEventListener("mousemove", this.adjustTooltipPosition);
}
}

/* We need to adjust the tooltips position manually because it is not positioned correctly when scrolling down.
* This fix was suggested by
* https://stackoverflow.com/questions/52755733/google-charts-tooltips-have-wrong-position-when-inside-a-scrolling-container.
* The fix is modified so that it sets the tooltip directly next to the mouse. This is done by using the total
* coordinates of the mouse of the whole window (clientX/Y) and manipulating the style of the tooltip directly. */
applyTooltipPositioningFix = () => {
MichaelBuessemeyer marked this conversation as resolved.
Show resolved Hide resolved
// TimeLineGraph is the name of the chart given by the library.
this.chartScrollElement = document.querySelector(
"#TimeLineGraph > div > div:first-child > div",
);
if (this.chartScrollElement) {
this.chartScrollElement.addEventListener("mousemove", this.adjustTooltipPosition);
}
};

adjustTooltipPosition = (event: MouseEvent) => {
const tooltip = document.getElementsByClassName("google-visualization-tooltip")[0];
if (tooltip != null) {
const { clientX, clientY } = event;
const [clientWidth, clientHeight] = getWindowBounds();
const { offsetHeight, offsetWidth } = tooltip;
if (clientY + offsetHeight >= clientHeight) {
// The tooltip needs to be displayed above the mouse.
tooltip.style.top = `${clientY - offsetHeight}px`;
} else {
// The tooltip can be displayed below the mouse.
tooltip.style.top = `${clientY}px`;
}
if (clientX + offsetWidth >= clientWidth) {
// The tooltip needs to be displayed on the left side of the mouse.
tooltip.style.left = `${clientX - offsetWidth - 5}px`;
} else {
// The tooltip needs to be displayed on the right side of the mouse.
tooltip.style.left = `${clientX + 15}px`;
}
// We need to make the tooltip visible again after adjusting the position.
// It is initially invisible as it is mispositioned by the library and would then "beam" to the corrected
// position. We want to avoid that "beaming" behaviour.
tooltip.style.visibility = "visible";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The "beaming" behavior e.g. happens in Firefox for me. Chrome works fine even without the workaround.
If you are looking where the tooltip is set to be invisible at the beginning: it is done in the google-charts-overwrites.less

}
};

render() {
const { columns, rows, timeAxisFormat, dateRange } = this.props;

const { applyTooltipPositioningFix } = this;

return (
<Chart
chartType="Timeline"
columns={columns}
rows={rows}
options={{
timeline: { singleColor: "#108ee9" },
// Workaround for google-charts bug, see https://github.com/scalableminds/webknossos/pull/3772
hAxis: {
format: timeAxisFormat,
minValue: dateRange[0].toDate(),
maxValue: dateRange[1].toDate(),
},
allowHtml: true,
}}
graph_id="TimeLineGraph"
chartPackages={["timeline"]}
width="100%"
height="600px"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@philippotto: We need a fixed height as the workaround we found, that displays the chart at full height, leaves out the whole y-axis with the actual dates. Having a fixed height solves this.
I also shortly tried to force the y-axis to be rendered below the chart but failed :/

legend_toggle
chartEvents={[
{
eventName: "ready",
callback() {
// After the whole chart is drawn, we can now apply the position fixing workaround.
applyTooltipPositioningFix();
},
},
]}
/>
);
}
}
59 changes: 26 additions & 33 deletions frontend/javascripts/admin/time/time_line_view.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// @flow
import { Chart } from "react-google-charts";
import { Select, Card, Form, Row, Col, DatePicker } from "antd";
import { Select, Card, Form, Row, Col, DatePicker, Spin } from "antd";
import * as React from "react";
import ReactDOMServer from "react-dom/server";
import { connect } from "react-redux";
Expand All @@ -9,12 +8,17 @@ import moment from "moment";
import FormattedDate from "components/formatted_date";
import { type OxalisState } from "oxalis/store";
import type { APIUser, APITimeTracking } from "admin/api_flow_types";
import { formatMilliseconds, formatDurationToHoursAndMinutes } from "libs/format_utils";
import { formatMilliseconds, formatDurationToMinutesAndSeconds } from "libs/format_utils";
import { isUserAdmin } from "libs/utils";
import { getEditableUsers, getTimeTrackingForUser } from "admin/admin_rest_api";
import Toast from "libs/toast";
import messages from "messages";
import { enforceActiveUser } from "oxalis/model/accessors/user_accessor";
import TimeTrackingChart, {
type DateRange,
type ColumnDefinition,
type RowContent,
} from "./time_line_chart_view";

const FormItem = Form.Item;
const { Option } = Select;
Expand All @@ -29,8 +33,6 @@ type TimeTrackingStats = {
averageTimePerTask: number,
};

type DateRange = [moment$Moment, moment$Moment];

type StateProps = {|
activeUser: APIUser,
|};
Expand All @@ -43,6 +45,7 @@ type State = {
dateRange: DateRange,
timeTrackingData: Array<APITimeTracking>,
stats: TimeTrackingStats,
isLoading: boolean,
};

class TimeLineView extends React.PureComponent<Props, State> {
Expand All @@ -56,6 +59,7 @@ class TimeLineView extends React.PureComponent<Props, State> {
numberTasks: 0,
averageTimePerTask: 0,
},
isLoading: false,
};

componentDidMount() {
Expand All @@ -73,6 +77,7 @@ class TimeLineView extends React.PureComponent<Props, State> {
}

async fetchTimeTrackingData() {
this.setState({ isLoading: true });
if (this.state.user != null) {
/* eslint-disable react/no-access-state-in-setstate */
const timeTrackingData = await getTimeTrackingForUser(
Expand All @@ -84,6 +89,7 @@ class TimeLineView extends React.PureComponent<Props, State> {
this.setState({ timeTrackingData }, this.calculateStats);
/* eslint-enable react/no-access-state-in-setstate */
}
this.setState({ isLoading: false });
}

calculateStats() {
Expand Down Expand Up @@ -136,10 +142,10 @@ class TimeLineView extends React.PureComponent<Props, State> {
getTooltipForEntry(taskId: string, start: Date, end: Date) {
const isSameDay = start.getUTCDate() === end.getUTCDate();
const duration = end - start;
const durationAsString = formatDurationToHoursAndMinutes(duration);
const durationAsString = formatDurationToMinutesAndSeconds(duration);
const dayFormatForMomentJs = "DD, MMM, YYYY";
const tooltip = (
<div className="google-charts-tooltip">
<div>
<div className="highlighted">
Task ID: {taskId}
<div className="striped-border" />
Expand Down Expand Up @@ -168,7 +174,7 @@ class TimeLineView extends React.PureComponent<Props, State> {
</td>
</tr>
<tr>
<td className="highlighted">Duration:</td>
<td className="highlighted">Duration (min:sec):</td>
<td>{durationAsString}</td>
</tr>
</tbody>
Expand All @@ -179,7 +185,7 @@ class TimeLineView extends React.PureComponent<Props, State> {
}

render() {
const columns = [
const columns: Array<ColumnDefinition> = [
{ id: "AnnotationId", type: "string" },
// This label columns is somehow needed to make the custom tooltip work.
// See https://developers.google.com/chart/interactive/docs/gallery/timeline#customizing-tooltips.
Expand All @@ -189,9 +195,9 @@ class TimeLineView extends React.PureComponent<Props, State> {
{ id: "End", type: "date" },
];

const { dateRange } = this.state;
const timeTrackingRowGrouped = []; // shows each time span grouped by annotation id
const timeTrackingRowTotal = []; // show all times spans in a single row
const { dateRange, isLoading, timeTrackingData } = this.state;
const timeTrackingRowGrouped: Array<RowContent> = []; // shows each time span grouped by annotation id
const timeTrackingRowTotal: Array<RowContent> = []; // show all times spans in a single row

const totalSumColumnLabel = "Sum Tracking Time";

Expand Down Expand Up @@ -292,35 +298,22 @@ class TimeLineView extends React.PureComponent<Props, State> {
</Row>
</Card>

<div style={{ marginTop: 20 }}>
{this.state.timeTrackingData.length > 0 ? (
<Chart
chartType="Timeline"
<div style={{ marginTop: 20 }} />
<Spin size="large" spinning={isLoading}>
{timeTrackingData.length > 0 ? (
<TimeTrackingChart
columns={columns}
rows={rows}
options={{
timeline: { singleColor: "#108ee9" },
// Workaround for google-charts bug, see https://github.com/scalableminds/webknossos/pull/3772
hAxis: {
format: timeAxisFormat,
minValue: dateRange[0].toDate(),
maxValue: dateRange[1].toDate(),
},
allowHtml: true,
tooltip: { isHtml: true },
}}
graph_id="TimeLineGraph"
chartPackages={["timeline"]}
width="100%"
height="600px"
legend_toggle
timeAxisFormat={timeAxisFormat}
dateRange={dateRange}
timeTrackingData={timeTrackingData}
/>
) : (
<div style={{ textAlign: "center" }}>
No Time Tracking Data for the Selected User or Date Range.
</div>
)}
</div>
</Spin>
</div>
);
}
Expand Down
15 changes: 5 additions & 10 deletions frontend/javascripts/libs/format_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,18 +98,13 @@ export function formatSeconds(durationSeconds: number): string {
return timeString;
}

export function formatDurationToHoursAndMinutes(
durationInMillisecons: number,
minMinutes: number = 1,
) {
export function formatDurationToMinutesAndSeconds(durationInMillisecons: number) {
// Moment does not provide a format method for durations, so we have to do it manually.
const duration = moment.duration(durationInMillisecons);
const minutesAsString = `${duration.minutes() < 10 ? 0 : ""}${Math.max(
duration.minutes(),
minMinutes,
)}`;
const hoursAsString = `${duration.hours() < 10 ? 0 : ""}${duration.hours()}`;
return `${hoursAsString}:${minutesAsString}`;
const minuteDuration = duration.minutes() + 60 * duration.hours();
const minutesAsString = `${minuteDuration < 10 ? 0 : ""}${minuteDuration}`;
const hoursAsSeconds = `${duration.seconds() < 10 ? 0 : ""}${duration.seconds()}`;
return `${minutesAsString}:${hoursAsSeconds}`;
}

export function formatHash(id: string): string {
Expand Down
23 changes: 23 additions & 0 deletions frontend/javascripts/libs/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,29 @@ export function getIsInIframe() {
}
}

export function getWindowBounds(): [number, number] {
// Function taken from https://stackoverflow.com/questions/3333329/javascript-get-browser-height.
let width = 0;
let height = 0;
if (typeof window.innerWidth === "number") {
// Non-IE
width = window.innerWidth;
height = window.innerHeight;
} else if (
document.documentElement &&
(document.documentElement.clientWidth || document.documentElement.clientHeight)
) {
// IE 6+ in 'standards compliant mode'
width = document.documentElement.clientWidth;
height = document.documentElement.clientHeight;
} else if (document.body && (document.body.clientWidth || document.body.clientHeight)) {
// IE 4 compatible
width = document.body.clientWidth;
height = document.body.clientHeight;
}
return [width, height];
}

export function disableViewportMetatag() {
const viewport = document.querySelector("meta[name=viewport]");
if (!viewport) {
Expand Down
4 changes: 4 additions & 0 deletions frontend/stylesheets/_google-charts-overwrites.less
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ div.google-visualization-tooltip {
font-weight: normal;
font-size: 13px;
border-radius: 6px;
position: fixed;
// We need to set the visibility to hidden as the tooltip is initially mispositioned.
// After adjusting the position, we set the visibility to visible again (@see TimeLineChart).
visibility: hidden;
.highlighted {
font-weight: bold;
}
Expand Down