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

Update Applications Table #1903

Merged
merged 30 commits into from
Apr 3, 2023
Merged

Update Applications Table #1903

merged 30 commits into from
Apr 3, 2023

Conversation

joepavitt
Copy link
Contributor

@joepavitt joepavitt commented Mar 30, 2023

Description

Correct me if this isn't the branch to be PRing back into @Pezmc. This also includes a merge of main so I could get the other UI work I'd done, hence the ridiculous number of files changed and commits, not sure how best to coordinate branches here, I can continue to pull updates from your feat-1735-expose-applications branch and then this PR could be the primary merge to main?

Updates:

  • Remove Team > Overview Page
  • Rework Applications table to show nested Instances

Still to do:

  • Update E2E Tests
  • Possible change of layout of table (see comments below)
  • Cannot render the "Open Editor" button as that is currently missing from the instances list for the /applications/<application-id>/instances API endpoint.

Related Issue(s)

Closes #1851

Checklist

# Conflicts:
#	frontend/src/pages/application/Settings.vue
#	frontend/src/pages/device/Overview.vue
#	frontend/src/pages/instance/index.vue
#	frontend/src/pages/project/index.vue
#	frontend/src/pages/team/Applications.vue
#	frontend/src/pages/team/routes.js
@joepavitt
Copy link
Contributor Author

joepavitt commented Mar 30, 2023

First pass, in order to align with the design resented here:

Screenshot 2023-03-30 at 16 45 13

This doesn't look very good when it's just a 1:1 Application > Instance, although I don't mind how it is in the design linked.

As such, going to iterate on this to the below, first mentioned here

Screenshot 2023-03-30 at 16 19 46

@joepavitt joepavitt changed the title Feat 1851 applications table Update Applications Table Mar 30, 2023
@joepavitt
Copy link
Contributor Author

joepavitt commented Mar 30, 2023

Dev'd up the alternative view, it uses up a lot more vertical space, but think it breaks down the hierarchy better. This is quite an unlikely scenario, where they have 6 different applications, and only one instance in each, vertical space is used more effectively for multiple instance within a given application due to where the padding lives.

In both scenarios, the duplication of Instance/Application name makes it trickier to digest too

Screen.Recording.2023-03-30.at.18.28.45.mov

@Pezmc
Copy link
Contributor

Pezmc commented Mar 31, 2023

In both scenarios, the duplication of Instance/Application name makes it trickier to digest too

This shouldn't apply any more, create new instances in your applications and delete the ones with the duplicate names (and/or create new applications).

@joepavitt
Copy link
Contributor Author

joepavitt commented Mar 31, 2023

This shouldn't apply any more, create new instances in your applications and delete the ones with the duplicate names (and/or create new applications).

Appreciate that - didn;t see an API (exposed in the application.js service) to update name of an existing Application? If there is an API for it, I'll just add the function to the service and functionality into UI

@Pezmc
Copy link
Contributor

Pezmc commented Mar 31, 2023

Renaming is not available yet, create new ones! The API is there for it though, see the application route on the backend.

It's set as a follow up on #1735

@joepavitt
Copy link
Contributor Author

If the API is there (I'll go digging in a second) - I'll just add it front-end - 10 mins of work.

@Pezmc Pezmc changed the base branch from feat-1735-expose-applications to main March 31, 2023 09:54
@Pezmc Pezmc changed the base branch from main to feat-1735-expose-applications March 31, 2023 09:55
@joepavitt
Copy link
Contributor Author

All development work now done - just tests remaining which I will get to next week (Tuesday)

@joepavitt joepavitt marked this pull request as ready for review April 1, 2023 10:45
@Pezmc
Copy link
Contributor

Pezmc commented Apr 3, 2023

@joepavitt after the other branches merged to main, I will do a rebase and then review this branch

@joepavitt
Copy link
Contributor Author

This has a merge from main and regular merge from your branch that this merges to. A lot of the difference here will come from the diff between main and your branch, so may be worth pulling latest main into yours to get the correct difference here?

Alternatively, we can switch this PR to point to main and merge your PR independently (and first)

Base automatically changed from feat-1735-expose-applications to main April 3, 2023 11:03
Copy link
Contributor

@Pezmc Pezmc left a comment

Choose a reason for hiding this comment

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

Addressed my comments since Joe is out

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

Successfully merging this pull request may close these issues.

Update "Applications" Table
2 participants