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

✨ Add app name filter to archetypes page & drawer link #1673

Merged
merged 2 commits into from
Feb 22, 2024

Conversation

ibolton336
Copy link
Member

@ibolton336 ibolton336 commented Jan 29, 2024

There was a suggestion to update the drawer chips to a link since as the list of chips grows, it will not be sustainable to show all chips in drawer. The change here moves to a pre-filter for the archetypes category in the applications table which is linkable from the archetype drawer.
https://issues.redhat.com/browse/MTA-2283

Screenshot 2024-02-21 at 4 51 48 PM

@ibolton336 ibolton336 force-pushed the scalable-drawer-links branch from 5f85287 to 8324593 Compare January 29, 2024 22:09
@ibolton336 ibolton336 marked this pull request as ready for review February 14, 2024 19:01
@ibolton336 ibolton336 requested a review from sjd78 February 14, 2024 19:01
Copy link
Member

@sjd78 sjd78 left a comment

Choose a reason for hiding this comment

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

On the PR description,

  • What part of 0.3.1 priorities #1672 is this PR addressing?
  • Can you add a short caption for the screen shots or add an arrow to show what has changed? Relying on my memory of how something currently looks, or trying to run the app to that point with similar data, to decipher the new screenshot is rough to my less-than-ideally-caffeinated brain.

All of the code changes look ok. Really my apprehension is with swapping out a list of archetypes for a single link. Feels like a loss of information in the details drawer.

client/public/locales/en/translation.json Outdated Show resolved Hide resolved
Comment on lines +231 to +232
key: applicationName,
value: applicationName,
Copy link
Member

Choose a reason for hiding this comment

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

Having the key and the value for the selectOptions be the same is currently annoying me for no good reason. I mean as long as the key value matches up with what is returned from getItemValue() all is well.

@ibolton336
Copy link
Member Author

On the PR description,

  • What part of 0.3.1 priorities #1672 is this PR addressing?
  • Can you add a short caption for the screen shots or add an arrow to show what has changed? Relying on my memory of how something currently looks, or trying to run the app to that point with similar data, to decipher the new screenshot is rough to my less-than-ideally-caffeinated brain.

Updated description. Sorry about that!

All of the code changes look ok. Really my apprehension is with swapping out a list of archetypes for a single link. Feels like a loss of information in the details drawer.

The concern with the chips in drawer was scalability. Maybe we can find a happy medium to keep some sort of chip list / detail in drawer along with the link.

@sjd78
Copy link
Member

sjd78 commented Feb 16, 2024

All of the code changes look ok. Really my apprehension is with swapping out a list of archetypes for a single link. Feels like a loss of information in the details drawer.

The concern with the chips in drawer was scalability. Maybe we can find a happy medium to keep some sort of chip list / detail in drawer along with the link.

Sticking with using labels to display the archetypes, this looks promising: https://www.patternfly.org/components/label#label-group-with-overflow

@ibolton336
Copy link
Member Author

All of the code changes look ok. Really my apprehension is with swapping out a list of archetypes for a single link. Feels like a loss of information in the details drawer.

The concern with the chips in drawer was scalability. Maybe we can find a happy medium to keep some sort of chip list / detail in drawer along with the link.

Sticking with using labels to display the archetypes, this looks promising: https://www.patternfly.org/components/label#label-group-with-overflow

@JustinXHale Looking for input here. I think it was Ramon who requested the link instead of chips here since there may be 100s of tags. Even with the overflow, that might be asking too much of our side drawer to render all of those chips in a readable manner.

@JustinXHale
Copy link
Member

All of the code changes look ok. Really my apprehension is with swapping out a list of archetypes for a single link. Feels like a loss of information in the details drawer.

The concern with the chips in drawer was scalability. Maybe we can find a happy medium to keep some sort of chip list / detail in drawer along with the link.

Sticking with using labels to display the archetypes, this looks promising: https://www.patternfly.org/components/label#label-group-with-overflow

@JustinXHale Looking for input here. I think it was Ramon who requested the link instead of chips here since there may be 100s of tags. Even with the overflow, that might be asking too much of our side drawer to render all of those chips in a readable manner.

Give me a couple days to look into this. I need to pay with some designs and see how we can take scalability into account when there can be a 100 tags

@sjd78
Copy link
Member

sjd78 commented Feb 16, 2024

@JustinXHale Looking for input here. I think it was Ramon who requested the link instead of chips here since there may be 100s of tags. Even with the overflow, that might be asking too much of our side drawer to render all of those chips in a readable manner.

Give me a couple days to look into this. I need to pay with some designs and see how we can take scalability into account when there can be a 100 tags

Specifically for this PR, it is considering the display of the archetypes that an application is associated to. I would be surprised if there are 100 different archetypes for an application, even if the number isn't really limited. The drawer has 3 things for the application archetypes:

  • the associated list
  • from the associated list, which ones are assessed
  • from the associated list, which ones are reviewed

Maybe a table layout in another tab like we have in dependency detail drawer?

Maybe it is worth thinking about how similar the application drawer's details/archetypes section and tags tabs should look?

cc: @JustinXHale

@ibolton336
Copy link
Member Author

@JustinXHale Looking for input here. I think it was Ramon who requested the link instead of chips here since there may be 100s of tags. Even with the overflow, that might be asking too much of our side drawer to render all of those chips in a readable manner.

Give me a couple days to look into this. I need to pay with some designs and see how we can take scalability into account when there can be a 100 tags

Specifically for this PR, it is considering the display of the archetypes that an application is associated to. I would be surprised if there are 100 different archetypes for an application, even if the number isn't really limited. The drawer has 3 things for the application archetypes:

  • the associated list
  • from the associated list, which ones are assessed
  • from the associated list, which ones are reviewed

Wondering if we should consider the inverse case - applications for a given archetype being > 100 applications:

Screenshot 2024-02-19 at 4 05 50 PM Screenshot 2024-02-19 at 4 02 04 PM

Maybe a table layout in another tab like we have in dependency detail drawer?

Like this?

Screenshot 2024-02-19 at 4 18 03 PM

Maybe it is worth thinking about how similar the application drawer's details/archetypes section and tags tabs should look?

Yeah I am not sure what we expect to see for tags. Currently we have this layout which already seems a bit stretched for an app with 40 tags:
Screenshot 2024-02-19 at 4 20 58 PM
Screenshot 2024-02-19 at 4 20 53 PM

cc: @JustinXHale

@JustinXHale
Copy link
Member

@ibolton336 i think a label group is great. that was my first thought, just wasnt sure if it would work if its 100 labels. But if that wont be an issue, i think the pattern is perfectly acceptable. We probably need to think about after how many labels to we want to show the more tag, i'm thinking 5.

@ibolton336 ibolton336 force-pushed the scalable-drawer-links branch 3 times, most recently from 4ebdfb3 to da72937 Compare February 21, 2024 21:53
@ibolton336
Copy link
Member Author

Latest changes move the code for link instead of label list from the app drawer to the archetype drawer as outlined here:
https://issues.redhat.com/browse/MTA-2283

@ibolton336 ibolton336 force-pushed the scalable-drawer-links branch from da72937 to 9a73a89 Compare February 22, 2024 14:02
@ibolton336 ibolton336 force-pushed the scalable-drawer-links branch from 9a73a89 to b8f74f1 Compare February 22, 2024 14:02
Copy link
Member

@djzager djzager left a comment

Choose a reason for hiding this comment

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

ack

@ibolton336 ibolton336 merged commit 4cdd553 into konveyor:main Feb 22, 2024
6 checks passed
@ibolton336 ibolton336 deleted the scalable-drawer-links branch February 22, 2024 15:19
ibolton336 added a commit that referenced this pull request Feb 29, 2024
There was a suggestion to update the drawer chips to a link since as the
list of chips grows, it will not be sustainable to show all chips in
drawer. The change here moves to a pre-filter for the archetypes
category in the applications table which is linkable from the archetype
drawer.
https://issues.redhat.com/browse/MTA-2283

<img width="1677" alt="Screenshot 2024-02-21 at 4 51 48 PM"
src="https://github.com/konveyor/tackle2-ui/assets/11218376/f787e32b-df98-48de-92a8-91521059df69">

---------

Signed-off-by: ibolton336 <[email protected]>
Signed-off-by: Ian Bolton <[email protected]>
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.

4 participants