feat: Add app list and details page views to navigation history (#7776) #7937
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7937 +/- ##
==========================================
- Coverage 41.53% 41.53% -0.01%
==========================================
Files 174 174
Lines 22736 22705 -31
==========================================
- Hits 9443 9430 -13
+ Misses 11949 11921 -28
- Partials 1344 1354 +10
Continue to review full report at Codecov.
|
keithchong
requested review from
pasha-codefresh,
reginapizza and
alexmt
and removed request for
reginapizza
ui/src/app/applications/components/application-details/application-details.tsx
Show resolved
Hide resolved
ui/src/app/applications/components/application-details/application-details.tsx
Show resolved
Hide resolved
ui/src/app/applications/components/applications-list/applications-list.tsx
Outdated
Show resolved
Hide resolved
ui/src/app/applications/components/applications-list/applications-list.tsx
Show resolved
Hide resolved
keithchong
force-pushed
the
7776-AddAppListAndDetailsToNavigationHistory
branch
2 times, most recently
from
99c8f75
to
81e88bb
Compare
|
Rebased. Hi @alexmt or @pasha-codefresh, could either one of you review this and try it out? |
| className={`argo-table-list__row applications-list__entry applications-list__entry--health-${app.status.health.status} ${ | ||
| selectedApp === i ? 'applications-tiles__selected' : '' | ||
| }`}> | ||
| <div className='row' onClick={e => ctx.navigation.goto(`/applications/${app.metadata.name}`, {view: pref.appDetails.view}, {event: e})}> |
There was a problem hiding this comment.
The application details page already loads view preferences and uses the correct view type. As far as I understand there is no specify view type in query parameter here.
There was a problem hiding this comment.
Hi @alexmt, yeah, it loads and selects the proper view fine, initially. It's just that we need to properly register it in the navigation history. Try this, with and without the changes to this file, for comparison:
- Launch Argo CD UI in a new browser tab, with existing apps already created
- Drill down to the details view of an application
- Switch to another view of the app (eg network or pods)
- Press the Back button
--> It does not change the view (without the changes to this file)
There was a problem hiding this comment.
Actually. we need to force using the query parameter, otherwise, the view doesn't change because the current view is actually the preferred view.
ui/src/app/applications/components/applications-list/applications-list.tsx
Outdated
Show resolved
Hide resolved
ui/src/app/applications/components/application-details/application-details.tsx
Outdated
Show resolved
Hide resolved
…proj#7776) Signed-off-by: Keith Chong <kykchong@redhat.com>
keithchong
force-pushed
the
7776-AddAppListAndDetailsToNavigationHistory
branch
from
81e88bb
to
0946c36
Compare
Signed-off-by: Keith Chong kykchong@redhat.com
Fixes #7776
Both Application List page view and Details page views are added to the navigation history
Some points to consider:
A. Tab independence (For a specific situation when
viewis not a query parameter)—> App List page
—> App List page
—> The page refreshed to the Summary page. This is because it gets the last preferred view
With the fix, the tab will remain on the Tiles view. Launching Argo CD UI without view the query param will always show the Tiles view.
B. With the fix, Clicking on the Sync Windows link brings you to the Settings page and then you want to click on Back to go back to the details page.
C. With the fix, Back and Forward button 'history' will have new unique titles. eg. "Application List - Argo CD"
D. With the fix, the Tool bar title text is now dynamic, but if you want it to remain the same as before, we just have to revert the
titlechange.eg. original titles for the app list page and details page, respectively:
"APPLICATIONS" and "APPLICATION DETAILS"
With Fix:
"APPLICATIONS TILES", "APPLICATIONS LIST", "APPLICATIONS SUMMARY"
"APPLICATION DETAILS TREE", "APPLICATION DETAILS PODS", "APPLICATION DETAILS NETWORK", "APPLICATION DETAILS LIST"
E. Behaviour of the Back and Forward buttons on the App List page presented additional issues that do not exist in the details page.
F. Clicking on the
Applicationslink from the details page will bring you back to the Tiles view of the applications list page, because of point A.Note on DCO:
If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.
Checklist: