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

UI improvements on dev file registry web view #3629

Conversation

msivasubramaniaan
Copy link
Collaborator

@msivasubramaniaan msivasubramaniaan commented Nov 27, 2023

Fixes: #3636

image

Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
Copy link

codecov bot commented Nov 27, 2023

Codecov Report

Attention: 34 lines in your changes are missing coverage. Please review.

Comparison is base (0d6dfaa) 33.41% compared to head (eedcd3a) 33.19%.
Report is 32 commits behind head on main.

❗ Current head eedcd3a differs from pull request most recent head f8775c5. Consider uploading reports for the commit f8775c5 to get more accurate results

Files Patch % Lines
src/webview/common-ext/createComponentHelpers.ts 8.10% 34 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3629      +/-   ##
==========================================
- Coverage   33.41%   33.19%   -0.23%     
==========================================
  Files          84       84              
  Lines        6096     6140      +44     
  Branches     1239     1254      +15     
==========================================
+ Hits         2037     2038       +1     
- Misses       4059     4102      +43     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
@vrubezhny
Copy link
Contributor

Just an opinion... I can't really say that I'm happy with the view layout...
Imho, the things by importance are:

  1. Title (however we can get rid of it as it actually duplicates the view title)
  2. List of Devfile registries
  3. Devfile list
  4. Search form
  5. Keyword filter

The previous layout was more or less consistent with this scheme, while the new one puts the keyword filter on top - like it's the most important thing, at the same time hiding the Devfile registry selection underneath and keeping the same space for the Devfile list (in my case its size is even reduced a bit)...

Personally I think that previous layout was looking better, the only thing I would change is that big header with a lot of empty space and title: I'd remove it giving more space to the Devfile list (again, I see the only 1.5-2 lines in the list which is really no good. Ideally if we can show all 6 items reserved to be on each page.. I know this is almost impossible, given the size of description text for each item and text size (which should be clearly visible), but still the more items we show here - the better).

IMHO

@datho7561
Copy link
Collaborator

I like the direction of this PR. Removing the title to better use the space is a great idea. There are a few changes that I think would be nice, though:

  • In the Devfile registry view, there is no gap at the top of the page, but in the Components view, there is a gap. It would be nice if this was consistent (either both have gaps or neither has a gap)
  • The "Back" button and the information box explaining what a Devfile is are hidden when opening the Devfile search from "Create Component". Removing the gap at the top of the page as mentioned in the first point should fix this.
  • When you click "Show More", the leftmost column expands a little. I think it would be a good idea to add some padding to the unexpanded version of the list of tags so that it doesn't shift when you expand it. The width of the scroll bar is var(--vscode-text-width) (i.e. the value of the CSS variable --vscode-text-width)

Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
@msivasubramaniaan msivasubramaniaan marked this pull request as ready for review November 30, 2023 12:02
@mohitsuman
Copy link
Collaborator

I like the direction of this PR. Removing the title to better use the space is a great idea. There are a few changes that I think would be nice, though:

  • In the Devfile registry view, there is no gap at the top of the page, but in the Components view, there is a gap. It would be nice if this was consistent (either both have gaps or neither has a gap)

All the views should e consistent, so we should be removing the gap. The idea is to utilise the real estate in the layout.

  • The "Back" button and the information box explaining what a Devfile is are hidden when opening the Devfile search from "Create Component". Removing the gap at the top of the page as mentioned in the first point should fix this.

@msivasubramaniaan is the screenshot in the description latest of the UX changes ?

  • When you click "Show More", the leftmost column expands a little. I think it would be a good idea to add some padding to the unexpanded version of the list of tags so that it doesn't shift when you expand it. The width of the scroll bar is var(--vscode-text-width) (i.e. the value of the CSS variable --vscode-text-width)

+1 to this.

Copy link
Collaborator

@mohitsuman mohitsuman left a comment

Choose a reason for hiding this comment

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

Few Changes in the layout:

  • Make search bar height smaller and placeholder inside it at center
  • Pagination items at the center of search bar alignment
  • Move Tags to the right and reduce logo size, to adjust more entries in the layout.
  • Show the Tags Filter in same order as we show The registries
  • Adjust the devfile yaml theme based on VSCode Light and Dark Theme

Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
@msivasubramaniaan
Copy link
Collaborator Author

Few Changes in the layout:

  • Make search bar height smaller and placeholder inside it at center
  • Pagination items at the center of search bar alignment
  • Move Tags to the right and reduce logo size, to adjust more entries in the layout.
  • Show the Tags Filter in same order as we show The registries
  • Adjust the devfile yaml theme based on VSCode Light and Dark Theme

Addressed all the mentioned items. Here is the latest UI:
image

Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
@datho7561
Copy link
Collaborator

Imho, the things by importance are:

  1. Title (however we can get rid of it as it actually duplicates the view
  2. List of Devfile registries
  3. Devfile list
    ...

(from Victor)

I missed this the first time I reviewed the PR, but I also agree that the Devfile Registry checkboxes should appear above the Tag checkboxes

Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
@msivasubramaniaan
Copy link
Collaborator Author

@datho7561 and @vrubezhny

Thanks for the review. I just fixed the issue identified by @vrubezhny please take a look.

@mohitsuman FYI I moved the dev file registry selection on top.

Copy link
Collaborator

@datho7561 datho7561 left a comment

Choose a reason for hiding this comment

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

I like that we can show more Devfiles in one page and I like the change to support light themes for the YAML viewer. I'm not enthusiastic about most of the other changes. It seems to work properly.

test/ui/suite/devfileRegistries.ts Show resolved Hide resolved
Copy link
Contributor

@vrubezhny vrubezhny left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks!

@datho7561
Copy link
Collaborator

still cannot merge?

@mohitsuman
Copy link
Collaborator

@msivasubramaniaan please share the latest UI look and also the UI for devfile yaml in both light and dark theme.

@msivasubramaniaan
Copy link
Collaborator Author

@mohitsuman FYR,

image
image
image

Copy link
Collaborator

@mohitsuman mohitsuman left a comment

Choose a reason for hiding this comment

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

lgtm

@mohitsuman mohitsuman merged commit ff12285 into redhat-developer:main Dec 7, 2023
4 checks passed
@msivasubramaniaan msivasubramaniaan deleted the devfile-registry-ui-improvements branch December 7, 2023 14:21
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 UX for Devfile Registry View
4 participants