bbovenzi commented on code in PR #74039:
URL: https://github.com/apache/airflow/pull/74039#discussion_r4158415352
##########
airflow-core/src/airflow/ui/src/components/TriggerDag/TriggerDAGButton.tsx:
##########
@@ -92,84 +113,84 @@ export const TriggerDAGButton = ({
onClose();
};
- // If there's a selected DAG Run with config, show menu with options
- if (selectedDagRun?.conf !== undefined) {
- return (
- <Box>
- <Menu.Root>
- <Tooltip content={translate("triggerDag.manualRunDenied")}
disabled={!isManualRunDenied}>
+ // The main part always triggers in one click, as it did before the config
menu existed. When the
+ // selected run carried a config, a caret next to it offers re-triggering
with that config, instead
+ // of turning the whole button into a menu (which cost everyone an extra
click).
+ const triggerButton = withText ? (
+ <Button
+ aria-label={translate("triggerDag.title")}
+ data-testid="trigger-dag-button"
+ disabled={isManualRunDenied}
+ onClick={handleNormalTrigger}
+ variant={variant}
+ >
+ <FiPlay />
+ {translate("triggerDag.button")}
+ </Button>
+ ) : (
+ <IconButton
+ aria-label={translate("triggerDag.title")}
+ data-testid="trigger-dag-button"
+ disabled={isManualRunDenied}
+ onClick={handleNormalTrigger}
+ variant={variant}
+ >
+ <FiPlay />
+ </IconButton>
+ );
+
+ const triggerButtonWithTooltip = (
+ <Tooltip
+ content={isManualRunDenied ? translate("triggerDag.manualRunDenied") :
translate("triggerDag.button")}
+ disabled={withText ? !isManualRunDenied : undefined}
+ >
+ {triggerButton}
+ </Tooltip>
+ );
+
+ return (
+ <>
+ {selectedDagRun?.conf === undefined ? (
Review Comment:
conf isn't undefined once there is a selected dag run. it's usually an empty
object `{}` so we should compare against that too
##########
airflow-core/src/airflow/ui/src/components/TriggerDag/TriggerDAGButton.tsx:
##########
@@ -92,84 +113,84 @@ export const TriggerDAGButton = ({
onClose();
};
- // If there's a selected DAG Run with config, show menu with options
- if (selectedDagRun?.conf !== undefined) {
- return (
- <Box>
- <Menu.Root>
- <Tooltip content={translate("triggerDag.manualRunDenied")}
disabled={!isManualRunDenied}>
+ // The main part always triggers in one click, as it did before the config
menu existed. When the
+ // selected run carried a config, a caret next to it offers re-triggering
with that config, instead
+ // of turning the whole button into a menu (which cost everyone an extra
click).
+ const triggerButton = withText ? (
+ <Button
+ aria-label={translate("triggerDag.title")}
+ data-testid="trigger-dag-button"
+ disabled={isManualRunDenied}
+ onClick={handleNormalTrigger}
+ variant={variant}
+ >
+ <FiPlay />
+ {translate("triggerDag.button")}
+ </Button>
+ ) : (
+ <IconButton
+ aria-label={translate("triggerDag.title")}
+ data-testid="trigger-dag-button"
+ disabled={isManualRunDenied}
+ onClick={handleNormalTrigger}
+ variant={variant}
+ >
+ <FiPlay />
+ </IconButton>
+ );
+
+ const triggerButtonWithTooltip = (
+ <Tooltip
+ content={isManualRunDenied ? translate("triggerDag.manualRunDenied") :
translate("triggerDag.button")}
+ disabled={withText ? !isManualRunDenied : undefined}
+ >
+ {triggerButton}
+ </Tooltip>
+ );
+
+ return (
+ <>
+ {selectedDagRun?.conf === undefined ? (
+ triggerButtonWithTooltip
+ ) : (
+ <ButtonGroup attached>
Review Comment:
The border radius is still being applied to both buttons. That's probably
because we're mixing menu and a regular button. So we might need custom logic
to decide when the left/right border radius is needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
