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

Top level masonry refactor #771

Merged
merged 4 commits into from
Jul 26, 2023
Merged

Top level masonry refactor #771

merged 4 commits into from
Jul 26, 2023

Conversation

FrankFlitton
Copy link
Contributor

@FrankFlitton FrankFlitton commented May 30, 2023

TL;DR

Normalize how parts of pages are related in the DOM to allow for more reliable flex box behavior across screen sizes.

Added a top level layout component that can be substituted in the registry.

  • Screen elements resize better with less UI cut off by the window.
  • Mobile nav for smaller screen sizes (ie tiling window manager, 1/2 a monitor).
  • Standardize flex/flex grid within the app to be more reliable.
  • Easier to make and maintain UI components and app pages - composable via MUI's Grid framework.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

Refactored much of the top level code to separate the left nav from the router view. The UI is now flex box based with no top level components having fixed positioning.

Nav is now programmatically shown and hidden via a context state (with animation). The current approach is hardcoding routes with a wrapper.

During the refactor, it was discovered that the useProjects hook was using a cache system that would dump the payload when the route changed or a new project was selected. The components (left nav and main view) were remounted and created fresh caches on the route changes which masked the issue.

Tracking Issue

Remove the 'fixes' keyword if there will be multiple PRs to fix the linked issue

fixes https://github.com/flyteorg/flyte/issues/

Follow-up issue

NA

@FrankFlitton FrankFlitton force-pushed the top-level-masonary-rework branch from 3e93280 to 111c3f0 Compare June 2, 2023 03:21
@FrankFlitton FrankFlitton changed the title WIP: Top level masonry refactor Top level masonry refactor Jun 9, 2023
@FrankFlitton FrankFlitton force-pushed the top-level-masonary-rework branch 2 times, most recently from 028d305 to d3f6584 Compare June 26, 2023 17:18
@codecov
Copy link

codecov bot commented Jun 26, 2023

Codecov Report

Merging #771 (4f71f62) into master (16abb34) will decrease coverage by 1.28%.
The diff coverage is 26.45%.

❗ Current head 4f71f62 differs from pull request most recent head 1b4d1e2. Consider uploading reports for the commit 1b4d1e2 to get more accurate results

@@            Coverage Diff             @@
##           master     #771      +/-   ##
==========================================
- Coverage   66.90%   65.63%   -1.28%     
==========================================
  Files         501      504       +3     
  Lines       12293    12505     +212     
  Branches     2262     2324      +62     
==========================================
- Hits         8225     8208      -17     
- Misses       4068     4297     +229     
Files Changed Coverage Δ
packages/components/src/AppInfo/index.tsx 100.00% <ø> (ø)
...gurationProvider/ExternalConfigurationProvider.tsx 71.42% <ø> (ø)
packages/console/src/components/App/App.tsx 0.00% <0.00%> (ø)
...console/src/components/Executions/Tables/styles.ts 100.00% <ø> (ø)
...console/src/components/Launch/LaunchForm/styles.ts 100.00% <ø> (ø)
...src/components/Navigation/DefaultAppBarContent.tsx 0.00% <0.00%> (ø)
...kages/console/src/components/Navigation/NavBar.tsx 0.00% <0.00%> (ø)
...le/src/components/Navigation/ProjectNavigation.tsx 0.00% <0.00%> (-36.85%) ⬇️
...nsole/src/components/Navigation/SideNavigation.tsx 0.00% <0.00%> (-81.82%) ⬇️
...nsole/src/components/Navigation/TopLevelLayout.tsx 0.00% <0.00%> (ø)
... and 22 more

... and 3 files with indirect coverage changes

@@ -40,10 +42,12 @@ const useStyles = makeStyles<Theme, StyleProps>((theme: Theme) => ({
width: '100%',
flex: '1',
overflowY: 'scroll',
padding: `0 ${theme.spacing(2)}`,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
padding: `0 ${theme.spacing(2)}`,
padding: theme.spacing(0, 2),

4nalog
4nalog previously requested changes Jun 27, 2023
Copy link
Member

@4nalog 4nalog left a comment

Choose a reason for hiding this comment

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

left initial set of comments

@4nalog 4nalog force-pushed the top-level-masonary-rework branch from da79f35 to a7bcf4c Compare June 29, 2023 17:05
@4nalog 4nalog self-requested a review June 29, 2023 17:05
@FrankFlitton FrankFlitton force-pushed the top-level-masonary-rework branch from 1807168 to 381d3be Compare July 11, 2023 22:26
@FrankFlitton FrankFlitton marked this pull request as ready for review July 25, 2023 17:18
Signed-off-by: Frank Flitton <[email protected]>
@FrankFlitton FrankFlitton force-pushed the top-level-masonary-rework branch from 381d3be to cad695b Compare July 25, 2023 17:21
jsonporter
jsonporter previously approved these changes Jul 25, 2023
FrankFlitton and others added 2 commits July 25, 2023 17:43
* chore: new nav bar

Signed-off-by: Frank Flitton <[email protected]>

* chore: navbar quick links

Signed-off-by: Frank Flitton <[email protected]>

* chore: task, workflow, launchplans

Signed-off-by: Frank Flitton <[email protected]>

* chore: remove unused import

Signed-off-by: Frank Flitton <[email protected]>

* chore: fixup navbar integration

Signed-off-by: Frank Flitton <[email protected]>

* chore: fix linter

Signed-off-by: Frank Flitton <[email protected]>

* chore: execution async value

Signed-off-by: Frank Flitton <[email protected]>

* chore: fix types build fail

Signed-off-by: Frank Flitton <[email protected]>

* chore: async self link

Signed-off-by: Frank Flitton <[email protected]>

* chore: add some exports

Signed-off-by: Frank Flitton <[email protected]>

* chore: change to names export

Signed-off-by: Frank Flitton <[email protected]>

* chore: spelling

Signed-off-by: Carina Ursu <[email protected]>

* chore: fix timestamps

Signed-off-by: Carina Ursu <[email protected]>

* chore: fix file upload type failure

Signed-off-by: Frank Flitton <[email protected]>

* chore: fix Executions title not updating

Signed-off-by: Frank Flitton <[email protected]>

* chore: remove depricated types file for dropzone

Signed-off-by: Frank Flitton <[email protected]>

* chore: fix execution launch plan self link

Signed-off-by: Frank Flitton <[email protected]>

* chore: pass custom component

Signed-off-by: Frank Flitton <[email protected]>

* chore: fix named entity cases

Signed-off-by: Frank Flitton <[email protected]>

* chore: different custom component style

Signed-off-by: Frank Flitton <[email protected]>

* chore: dynamic breadcrumb hook with event dispatch

Signed-off-by: Frank Flitton <[email protected]>

* chore: dynamic breadcrumb hook with event dispatch

Signed-off-by: Frank Flitton <[email protected]>

* chore: project and domains link to dashboard

Signed-off-by: Frank Flitton <[email protected]>

* chore: add localstorage setting

Signed-off-by: Frank Flitton <[email protected]>

* chore: get localstorage setting

Signed-off-by: Frank Flitton <[email protected]>

* chore: named entities self links

Signed-off-by: Frank Flitton <[email protected]>

* chore: spacing and scaffolding

Signed-off-by: Frank Flitton <[email protected]>

* chore: grey header color

Signed-off-by: Frank Flitton <[email protected]>

* fix/dropdown breadcrumbs (#799)

fix: dropdown breadcrumbs

Signed-off-by: Soham Parekh <[email protected]>
Co-authored-by: Soham Parekh <[email protected]>

* chore: global inject without emotion

Signed-off-by: Frank Flitton <[email protected]>

* chore: generate id

Signed-off-by: Frank Flitton <[email protected]>

* chore: initial unit tests

Signed-off-by: Frank Flitton <[email protected]>

* chore: fix incorrect localstore value

Signed-off-by: Frank Flitton <[email protected]>

* chore: named entities execution tests

Signed-off-by: Frank Flitton <[email protected]>

* chore: domain and project util tests

Signed-off-by: Frank Flitton <[email protected]>

* chore: fix execution list page title 404 ing

Signed-off-by: Frank Flitton <[email protected]>

* chore: fix swipe left to go back

Signed-off-by: Frank Flitton <[email protected]>

* chore: feature flag breadcrumb UI content

Signed-off-by: Frank Flitton <[email protected]>

* chore: set flags from url

Signed-off-by: Frank Flitton <[email protected]>

* chore: set flags from exxternal config

Signed-off-by: Frank Flitton <[email protected]>

* chore: set feature flags from env

Signed-off-by: Frank Flitton <[email protected]>

* chore: get flags from external env

Signed-off-by: Frank Flitton <[email protected]>

* chore: better docs from breadcrumbs interface

Signed-off-by: Frank Flitton <[email protected]>

---------

Signed-off-by: Frank Flitton <[email protected]>
Signed-off-by: Carina Ursu <[email protected]>
Signed-off-by: Soham Parekh <[email protected]>
Co-authored-by: Carina Ursu <[email protected]>
Co-authored-by: etdotal <[email protected]>
Co-authored-by: Soham Parekh <[email protected]>
@FrankFlitton FrankFlitton dismissed 4nalog’s stale review July 25, 2023 21:56

Changes addressed in a different commit.

ursucarina
ursucarina previously approved these changes Jul 26, 2023
Copy link
Contributor

@ursucarina ursucarina left a comment

Choose a reason for hiding this comment

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

You might need to up the package versions for components, console and update website as well

jsonporter
jsonporter previously approved these changes Jul 26, 2023
Signed-off-by: Frank Flitton <[email protected]>
@FrankFlitton FrankFlitton dismissed stale reviews from jsonporter and ursucarina via 1b4d1e2 July 26, 2023 17:06
@jsonporter jsonporter merged commit fd145ec into master Jul 26, 2023
@jsonporter jsonporter deleted the top-level-masonary-rework branch July 26, 2023 17:11
@flyte-bot
Copy link
Collaborator

🎉 This PR is included in version 1.9.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants