-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
docs: Proposal to support multi-source apps in the UI (#17106) #17108
docs: Proposal to support multi-source apps in the UI (#17106) #17108
Conversation
Signed-off-by: Keith Chong <[email protected]>
d67687c
to
1320978
Compare
widgets to allow users to cycle (via pagination or combo selector?) through each source. There are API calls to retrieve | ||
the data for each source. | ||
|
||
<img height="50%" width="50%" src="images/new-sources-tab.png"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a huge fan of this specific rendering.
- Generally this specific widget is more commonly used at the bottom of a view (sometimes both top and bottom).
- I don't have a better proposal at this time.
- Sometimes drop-downs are used for this purpose, although there are definite downsides.
How many are likely? Just 3, or are 10, and 100 conceivable/likely?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jsoref, I think there will be advantages and disadvantages as well. In the appendix, I talked about collapsible cards where each card houses the source/input parameters. But it can get unwieldy if we try to support N-number of sources.
Maybe it could end up being a combination of collapsible cards and pagination. Note that there is some time required to load the source details. So potentially, we can defer loading until the page is turned. If all cards are in the one tab, then all sources need to be loaded.
From what I was told, from the original multi-source proposal, they discussed 10 sources. But don't know what users will do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, I agree. Good to see what scenarios there are and what people suggest. The Argo CD UI doesn't really have a 'representation' like that right now...I think. The closest thing I think is the list view of applications (and also the details list view of an application). The Applications cards represent the Source cards, but they need to be expandable and collapsible. The delete action will be on each card. And the New Source action will be at the top somewhere above the top card. So maybe for consistency, this is the way to go? I don't think it will take much effort to get this up and running.
Yeah, I agree that the IntelliJ (and also the VSCode) vertical view on the left is good, but this change will be more involved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be difficult to add enough information about the sources in the vertical view without making each item a bigger block with multiple fields. It might be good to have 10 sources per page and a collapsible card / pop-up modal for each source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not bad -- although it's kinda hard to read. Could you give it a ring and opacity or something so it's easier to see?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what I was referring to, where if you change the repo URL, or simply change the path within the repo, the input parameters and even the source type can change. So it's probably better to keep these values as separate sections. So perhaps we will need two Edit buttons, like in the screenshot below? As mentioned, for single source apps, these fields are shown in the Summary section, under a different editable section. If you change the values there, the parameters tab will be updated once you visit the tab.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm... Yeah, I think it makes sense to have two edit buttons because they're really very unrelated things
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The initial PR changes for 'read-only' mode is here. #17275.
A lot of it is refactoring. I think we can make more progress from these changes.
Co-authored-by: Josh Soref <[email protected]> Signed-off-by: Keith Chong <[email protected]>
Co-authored-by: Josh Soref <[email protected]> Signed-off-by: Keith Chong <[email protected]>
Co-authored-by: Josh Soref <[email protected]> Signed-off-by: Keith Chong <[email protected]>
Co-authored-by: Josh Soref <[email protected]> Signed-off-by: Keith Chong <[email protected]>
Co-authored-by: Josh Soref <[email protected]> Signed-off-by: Keith Chong <[email protected]>
Co-authored-by: Josh Soref <[email protected]> Signed-off-by: Keith Chong <[email protected]>
Co-authored-by: Josh Soref <[email protected]> Signed-off-by: Keith Chong <[email protected]>
Signed-off-by: Keith Chong <[email protected]>
The view should show one source at a time (similar to what the UI is doing now, which only shows one source), but with | ||
widgets to allow users to cycle (via pagination or combo selector?) through each source. There are API calls to retrieve | ||
the data for each source. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to clarify the proposal based on what was discussed above, does this make sense?
The view should show one source at a time (similar to what the UI is doing now, which only shows one source), but with | |
widgets to allow users to cycle (via pagination or combo selector?) through each source. There are API calls to retrieve | |
the data for each source. | |
The view should show all the different sources in individual collapsible widgets with pagination to allow users to cycle through each source. | |
There are API calls to retrieve the data for each source. | |
When there is only one source is the view, it will be expanded by default. If multiple sources are defined, they will be collapsed and the user will be able to expand them and paginate based on the number of items per page and page number. The default number of item per page should be 3 and saved in the user session. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @agaudreault . Looks good. Just one comment about pagination with 3 items per page. The paginate component has hardcoded values, with 5 items per page as the minimum, then, 10, 15, 20 and All, as choices. How about keeping it at 5 items per page as the minimum, because if we add 3 as a choice (as the simplest change, and ignoring an enhancement to provide custom values), it will change all the pages that use this paginate component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding the comment "There are API calls to retrieve the data for each source": I looked into this some more and I think for better performance and usability, it should load the source details 'on demand' when the source section is expanded. Like this:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I thought the default could have been configurable easily. I like the async loading, I was expecting the data to be already fetched, because it is part of the Application Manifest information, but if it needs to load something, async is definitely my preference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!!
@jsoref @ashutosh16 @agaudreault are there any concerns with merging the proposal? |
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can live with it
argoproj#17108) * docs: Proposal to support multi-source apps in the UI (argoproj#17106) Signed-off-by: Keith Chong <[email protected]> * Update docs/proposals/multiple-sources-for-applications-ui.md Co-authored-by: Josh Soref <[email protected]> Signed-off-by: Keith Chong <[email protected]> * Update docs/proposals/multiple-sources-for-applications-ui.md Co-authored-by: Josh Soref <[email protected]> Signed-off-by: Keith Chong <[email protected]> * Update docs/proposals/multiple-sources-for-applications-ui.md Co-authored-by: Josh Soref <[email protected]> Signed-off-by: Keith Chong <[email protected]> * Update docs/proposals/multiple-sources-for-applications-ui.md Co-authored-by: Josh Soref <[email protected]> Signed-off-by: Keith Chong <[email protected]> * Update docs/proposals/multiple-sources-for-applications-ui.md Co-authored-by: Josh Soref <[email protected]> Signed-off-by: Keith Chong <[email protected]> * Update docs/proposals/multiple-sources-for-applications-ui.md Co-authored-by: Josh Soref <[email protected]> Signed-off-by: Keith Chong <[email protected]> * Update docs/proposals/multiple-sources-for-applications-ui.md Co-authored-by: Josh Soref <[email protected]> Signed-off-by: Keith Chong <[email protected]> * Minor edits Signed-off-by: Keith Chong <[email protected]> --------- Signed-off-by: Keith Chong <[email protected]> Co-authored-by: Josh Soref <[email protected]>
argoproj#17108) * docs: Proposal to support multi-source apps in the UI (argoproj#17106) Signed-off-by: Keith Chong <[email protected]> * Update docs/proposals/multiple-sources-for-applications-ui.md Co-authored-by: Josh Soref <[email protected]> Signed-off-by: Keith Chong <[email protected]> * Update docs/proposals/multiple-sources-for-applications-ui.md Co-authored-by: Josh Soref <[email protected]> Signed-off-by: Keith Chong <[email protected]> * Update docs/proposals/multiple-sources-for-applications-ui.md Co-authored-by: Josh Soref <[email protected]> Signed-off-by: Keith Chong <[email protected]> * Update docs/proposals/multiple-sources-for-applications-ui.md Co-authored-by: Josh Soref <[email protected]> Signed-off-by: Keith Chong <[email protected]> * Update docs/proposals/multiple-sources-for-applications-ui.md Co-authored-by: Josh Soref <[email protected]> Signed-off-by: Keith Chong <[email protected]> * Update docs/proposals/multiple-sources-for-applications-ui.md Co-authored-by: Josh Soref <[email protected]> Signed-off-by: Keith Chong <[email protected]> * Update docs/proposals/multiple-sources-for-applications-ui.md Co-authored-by: Josh Soref <[email protected]> Signed-off-by: Keith Chong <[email protected]> * Minor edits Signed-off-by: Keith Chong <[email protected]> --------- Signed-off-by: Keith Chong <[email protected]> Co-authored-by: Josh Soref <[email protected]>
This is the proposal document for supporting multi-source applications in the UI
Fixes #17106
Checklist: