-
Notifications
You must be signed in to change notification settings - Fork 14.2k
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
Use same tooltip for Graph and Tree views for TaskInstances #8043
Conversation
Whops, just notcied the |
They had _similar_ info, but they weren't identical and should have been. To not include the 250kb of moment-timezone in more than one Webpack output bundle I changed how that is imported/handled. There may be a better way of doing this in webpack, but I couldn't find it, and this isn't horrible.
29cf07d
to
c12f4c1
Compare
const makeDateTimeHTML = (start, end) => { | ||
return ( | ||
`Started: ${start.format(defaultFormat)} <br> Ended: ${end.format(defaultFormat)} <br>` | ||
) | ||
}; | ||
|
||
export const generateTooltipDateTime = (startDate, endDate, dagTZ) => { | ||
const tzFormat = 'z (Z)'; | ||
const localTZ = moment.tz.guess(); | ||
startDate = moment.utc(startDate); | ||
endDate = moment.utc(endDate); | ||
dagTZ = dagTZ.toUpperCase(); | ||
|
||
// Generate UTC Start and End Date | ||
let tooltipHTML = '<br><strong>UTC</strong><br>'; | ||
tooltipHTML += makeDateTimeHTML(startDate, endDate); | ||
|
||
// Generate User's Local Start and End Date | ||
tooltipHTML += `<br><strong>Local: ${moment.tz(localTZ).format(tzFormat)}</strong><br>`; | ||
tooltipHTML += makeDateTimeHTML(startDate.local(), endDate.local()); | ||
|
||
// Generate DAG's Start and End Date | ||
if (dagTZ !== 'UTC' && dagTZ !== localTZ) { | ||
tooltipHTML += `<br><strong>DAG's TZ: ${moment.tz(dagTZ).format(tzFormat)}</strong><br>`; | ||
tooltipHTML += makeDateTimeHTML(startDate.tz(dagTZ), endDate.tz(dagTZ)); | ||
} | ||
|
||
return tooltipHTML | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved these fns in to a new "task-instances.js" as they are specific to there, and not general "datetime" utils.
@@ -16,8 +16,8 @@ | |||
* specific language governing permissions and limitations | |||
* under the License. | |||
*/ | |||
import { generateTooltipDateTime, converAndFormatUTC, secondsToString } from './datetime-utils'; | |||
import { escapeHtml } from './base'; | |||
// global tiTooltip, taskTip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Global as it's included in another file that is sourced.
If I used a normal import webpack would include it in two files.
They had _similar_ info, but they weren't identical and should have been. To not include the 250kb of moment-timezone in more than one Webpack output bundle I changed how that is imported/handled. There may be a better way of doing this in webpack, but I couldn't find it, and this isn't horrible. Co-Authored-By: Ash Berlin-Taylor <[email protected]>
They had _similar_ info, but they weren't identical and should have been. To not include the 250kb of moment-timezone in more than one Webpack output bundle I changed how that is imported/handled. There may be a better way of doing this in webpack, but I couldn't find it, and this isn't horrible. Co-Authored-By: Ash Berlin-Taylor <[email protected]>
They had similar info, but they weren't identical and should have
been.
To not include the 250kb of moment-timezone in more than one Webpack
output bundle I changed how that is imported/handled. There may be a
better way of doing this in webpack, but I couldn't find it, and this
isn't horrible.
Graph TI:
Before:
After:
(i.e. unchanged)
Tree TI:
Before:
After:
Tree DagRun
Before: BROKEN ON MASTER -- no tooltip was displayed! Oops.
After:
Issue link: WILL BE INSERTED BY boring-cyborg
Make sure to mark the boxes below before creating PR: [x]
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.
Read the Pull Request Guidelines for more information.