bbovenzi commented on a change in pull request #15007: URL: https://github.com/apache/airflow/pull/15007#discussion_r601543610
########## File path: airflow/ui/src/containers/AppContainer/AppNav.tsx ########## @@ -0,0 +1,120 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React from 'react'; +import { Link } from 'react-router-dom'; +import { Box } from '@chakra-ui/react'; +import { + FiActivity, + FiBookOpen, + FiSettings, + FiUsers, +} from 'react-icons/fi'; + +import { useAuthContext } from 'auth/context'; + +import { PipelineIcon } from 'utils/icons'; + +import AppNavBtn from './AppNavBtn'; + +interface Props { + bodyBg: string; + overlayBg: string; +} + +const AppNav: React.FC<Props> = ({ bodyBg, overlayBg }) => { + const { hasValidAuthToken } = useAuthContext(); + + const navItems = [ + { + label: 'Pipelines', + icon: PipelineIcon, + path: '/pipelines', + activePath: '/pipelines', + }, + { + label: 'Activity', + icon: FiActivity, + path: '/activity/event-logs', + activePath: '/activity', + }, + { + label: 'Config', + icon: FiSettings, + path: '/config', + activePath: '/config', + }, + { + label: 'access', + icon: FiUsers, + path: '/access', + activePath: '/access', + }, + { + label: 'Docs', + icon: FiBookOpen, + path: '/docs', + activePath: '/docs', + }, + ]; + + return ( + <Box + as="nav" + role="navigation" + width="56px" + backgroundColor={overlayBg} + borderRightWidth="1px" + borderRightColor={bodyBg} + display="flex" + flexDirection="column" + > + <Box + as={Link} + to="/" + aria-label="Back to home" + width="56px" + height="56px" + display="flex" + alignItems="center" + justifyContent="center" + _hover={{ + transformOrigin: '28px 28px', + }} + > + <svg width="40" height="40" viewBox="0 0 40 40" fill="none" xmlns="http://www.w3.org/2000/svg";> Review comment: I think it would be best if all `svg`s are their own components ########## File path: yarn.lock ########## @@ -0,0 +1,4 @@ +# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. Review comment: Is this file supposed to exist? ########## File path: airflow/ui/src/containers/AppContainer/AppHeader.tsx ########## @@ -0,0 +1,129 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React from 'react'; +import { Link } from 'react-router-dom'; +import dayjs from 'dayjs'; +import { + Avatar, + Box, + Button, + Flex, + Icon, + Menu, + MenuButton, + MenuDivider, + MenuList, + MenuItem, + useColorMode, + useColorModeValue, + Tooltip, +} from '@chakra-ui/react'; +import { + MdWbSunny, + MdBrightness2, + MdAccountCircle, + MdExitToApp, +} from 'react-icons/md'; +import { useAuthContext } from 'auth/context'; +import { ApacheAirflowIcon } from 'utils/icons'; + +interface Props { + bodyBg: string; + overlayBg: string; + breadcrumb?: React.ReactNode; +} + +const AppHeader: React.FC<Props> = ({ bodyBg, overlayBg, breadcrumb }) => { + const { toggleColorMode } = useColorMode(); + const now = dayjs(); + const headerHeight = '56px'; + const { hasValidAuthToken, logout } = useAuthContext(); + + const handleOpenTZ = () => window.alert('This will open time zone select modal!'); + + const handleOpenProfile = () => window.alert('This will take you to your user profile view.'); + + return ( + <Flex + as="header" + role="banner" + position="fixed" + width={`calc(100vw - ${headerHeight})`} + height={headerHeight} + zIndex={2} + align="center" + justifyContent="space-between" + py="2" + px="4" + backgroundColor={overlayBg} + borderBottomWidth="1px" + borderBottomColor={bodyBg} + > + {breadcrumb} + {!breadcrumb && ( + <Link to="/" aria-label="Back to home"> + <ApacheAirflowIcon /> + </Link> + )} + {hasValidAuthToken && ( + <Flex align="center"> + <Tooltip label="Change time zone" hasArrow> + {/* TODO: open modal for time zone update */} + <Button variant="ghost" mr="4" onClick={() => handleOpenTZ()}> + <Box + as="time" + dateTime={now.toString()} + fontSize="md" + > + {now.format('h:mmA Z')} + </Box> + </Button> + </Tooltip> + <Menu> + <MenuButton> + <Avatar name="Ryan Hamilton" size="sm" color="blue.900" bg="blue.200" /> + </MenuButton> + <MenuList placement="top-end"> + <MenuItem onClick={() => handleOpenProfile()}> Review comment: Nitpick `onClick={handleOpenProfile}` ########## File path: airflow/ui/.neutrinorc.js ########## @@ -42,6 +42,7 @@ module.exports = { neutrino.config.resolve.alias.set('utils', resolve(__dirname, 'src/utils')); neutrino.config.resolve.alias.set('auth', resolve(__dirname, 'src/auth')); neutrino.config.resolve.alias.set('components', resolve(__dirname, 'src/components')); + neutrino.config.resolve.alias.set('containers', resolve(__dirname, 'src/containers')); Review comment: Many of the containers are specific to a set of views. Would it not be better for them to live closer to them? (ie `/views/access/Container.tsx` is the AccessContainer. ########## File path: airflow/ui/src/containers/AppContainer/AppHeader.tsx ########## @@ -0,0 +1,129 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React from 'react'; +import { Link } from 'react-router-dom'; +import dayjs from 'dayjs'; +import { + Avatar, + Box, + Button, + Flex, + Icon, + Menu, + MenuButton, + MenuDivider, + MenuList, + MenuItem, + useColorMode, + useColorModeValue, + Tooltip, +} from '@chakra-ui/react'; +import { + MdWbSunny, + MdBrightness2, + MdAccountCircle, + MdExitToApp, +} from 'react-icons/md'; +import { useAuthContext } from 'auth/context'; +import { ApacheAirflowIcon } from 'utils/icons'; + +interface Props { + bodyBg: string; + overlayBg: string; + breadcrumb?: React.ReactNode; +} + +const AppHeader: React.FC<Props> = ({ bodyBg, overlayBg, breadcrumb }) => { + const { toggleColorMode } = useColorMode(); + const now = dayjs(); + const headerHeight = '56px'; + const { hasValidAuthToken, logout } = useAuthContext(); + + const handleOpenTZ = () => window.alert('This will open time zone select modal!'); + + const handleOpenProfile = () => window.alert('This will take you to your user profile view.'); + + return ( + <Flex + as="header" + role="banner" + position="fixed" + width={`calc(100vw - ${headerHeight})`} + height={headerHeight} + zIndex={2} + align="center" + justifyContent="space-between" + py="2" + px="4" + backgroundColor={overlayBg} + borderBottomWidth="1px" + borderBottomColor={bodyBg} + > + {breadcrumb} + {!breadcrumb && ( + <Link to="/" aria-label="Back to home"> + <ApacheAirflowIcon /> + </Link> + )} + {hasValidAuthToken && ( + <Flex align="center"> + <Tooltip label="Change time zone" hasArrow> + {/* TODO: open modal for time zone update */} + <Button variant="ghost" mr="4" onClick={() => handleOpenTZ()}> + <Box + as="time" + dateTime={now.toString()} + fontSize="md" + > + {now.format('h:mmA Z')} + </Box> + </Button> + </Tooltip> + <Menu> + <MenuButton> + <Avatar name="Ryan Hamilton" size="sm" color="blue.900" bg="blue.200" /> + </MenuButton> + <MenuList placement="top-end"> + <MenuItem onClick={() => handleOpenProfile()}> + <Icon as={MdAccountCircle} mr="2" /> + Your Profile + </MenuItem> + <MenuItem + onClick={toggleColorMode} Review comment: Nitpick: We can make this all one line `<MenuItem onClick={toggleColorMode}>` ########## File path: airflow/ui/src/views/Docs.tsx ########## @@ -18,12 +18,160 @@ */ import React from 'react'; -import { Center, Heading } from '@chakra-ui/react'; +import { + Box, + Button, + Flex, + Heading, + Icon, + Link, + List, + ListItem, + Text, + useColorModeValue, +} from '@chakra-ui/react'; +import { FaGithub } from 'react-icons/fa'; +import { FiExternalLink, FiGlobe } from 'react-icons/fi'; + +import AppContainer from 'containers/AppContainer'; const Docs: React.FC = () => ( - <Center height="100vh"> - <Heading>Docs</Heading> - </Center> + <AppContainer> + <Box mx="auto" my={8} maxWidth="900px"> + <Flex mt={8}> + <Box flex="1"> + <Heading as="h1">Documentation</Heading> + <Text mt={4}> + Apache Airflow Core, which includes webserver, scheduler, CLI and other components that + are needed for minimal Airflow installation. + </Text> + <Button + as="a" + href="https://airflow.apache.org/docs/apache-airflow/stable/index.html"; + variant="solid" + rightIcon={<FiExternalLink />} + mt={4} + target="_blank" + rel="noopener noreferrer" + > + Apache Airflow Docs + </Button> + </Box> + <Box ml={8} p={4} bg={useColorModeValue('gray.100', 'gray.700')} borderRadius="md"> + <Heading as="h3" size="sm">Links</Heading> + <List mt={4} spacing={2}> + <ListItem> + <Link href="https://airflow.apache.org/"; isExternal color="teal.500"> + <Icon as={FiGlobe} mr={1} /> + Apache Airflow Website + </Link> + </ListItem> + <ListItem> + <Link href="https://github.com/apache/airflow"; isExternal color="teal.500"> + <Icon as={FaGithub} mr={1} /> + apache/airflow on GitHub + </Link> + </ListItem> + </List> + </Box> + </Flex> + <Box mt={10}> + <Heading as="h3" size="lg">REST API Reference</Heading> + <Flex mt={4}> + <Button + as="a" + href="http://127.0.0.1:28080/api/v1/ui/"; + variant="outline" + rightIcon={<FiExternalLink />} + mr={2} + > + Swagger + </Button> + <Button + as="a" + href="http://127.0.0.1:28080/redoc"; + variant="outline" + rightIcon={<FiExternalLink />} + > + Redoc + </Button> + </Flex> + </Box> + <Box mt={10}> + <Heading as="h3" size="lg">Providers Packages</Heading> + <Text mt={4}> + Providers packages include integrations with third party integrations. + They are updated independently of the Apache Airflow core. + </Text> + + <List spacing={2} mt={4} style={{ columns: 3 }}> + <ListItem><Link href="https://airflow.apache.org/docs/apache-airflow-providers-amazon/stable/index.html"; isExternal color="teal.500">Amazon</Link></ListItem> Review comment: Could we store all this info as an array of objects we parse through? I think it would be easier to read that way. ########## File path: airflow/ui/src/views/Activity/XComs.tsx ########## @@ -0,0 +1,31 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React from 'react'; +import { Heading } from '@chakra-ui/react'; + +import ActivityContainer from 'containers/ActivityContainer'; + +const xcoms: React.FC = () => ( Review comment: This should be capitalized like all other components `<XComs />` ########## File path: airflow/ui/src/containers/AppContainer/AppHeader.tsx ########## @@ -0,0 +1,129 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React from 'react'; +import { Link } from 'react-router-dom'; +import dayjs from 'dayjs'; +import { + Avatar, + Box, + Button, + Flex, + Icon, + Menu, + MenuButton, + MenuDivider, + MenuList, + MenuItem, + useColorMode, + useColorModeValue, + Tooltip, +} from '@chakra-ui/react'; +import { + MdWbSunny, + MdBrightness2, + MdAccountCircle, + MdExitToApp, +} from 'react-icons/md'; +import { useAuthContext } from 'auth/context'; +import { ApacheAirflowIcon } from 'utils/icons'; + +interface Props { + bodyBg: string; + overlayBg: string; + breadcrumb?: React.ReactNode; +} + +const AppHeader: React.FC<Props> = ({ bodyBg, overlayBg, breadcrumb }) => { + const { toggleColorMode } = useColorMode(); + const now = dayjs(); + const headerHeight = '56px'; + const { hasValidAuthToken, logout } = useAuthContext(); + + const handleOpenTZ = () => window.alert('This will open time zone select modal!'); + + const handleOpenProfile = () => window.alert('This will take you to your user profile view.'); + + return ( + <Flex + as="header" + role="banner" + position="fixed" + width={`calc(100vw - ${headerHeight})`} + height={headerHeight} + zIndex={2} + align="center" + justifyContent="space-between" + py="2" + px="4" + backgroundColor={overlayBg} + borderBottomWidth="1px" + borderBottomColor={bodyBg} + > + {breadcrumb} + {!breadcrumb && ( + <Link to="/" aria-label="Back to home"> + <ApacheAirflowIcon /> + </Link> + )} + {hasValidAuthToken && ( + <Flex align="center"> + <Tooltip label="Change time zone" hasArrow> + {/* TODO: open modal for time zone update */} + <Button variant="ghost" mr="4" onClick={() => handleOpenTZ()}> Review comment: Nitpick: you can just pass the function as `onClick={handleOpenTZ}` -- 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. For queries about this service, please contact Infrastructure at: [email protected]
