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

feat: file grid #2057

Closed
wants to merge 37 commits into from
Closed

feat: file grid #2057

wants to merge 37 commits into from

Conversation

gavinbarron
Copy link
Member

Closes #

PR Type

  • Feature

Description of the changes

Adds a file grid view with per item context menus to perform file actions

PR checklist

  • Project builds (yarn build) and changes have been tested in at least two supported browsers (Edge + non-Chromium based browser)
  • All public APIs (classes, methods, etc) have been documented following the jsdoc syntax
  • Stories have been added and existing stories have been tested
  • Added appropriate documentation. Docs PR:
  • License header has been added to all new source files (yarn setLicense)
  • Contains NO breaking changes

Other information

@ghost
Copy link

ghost commented Feb 23, 2023

Thank you for creating a Pull Request @gavinbarron.

This is a checklist for the PR reviewer(s) to complete before approving and merging this PR:

  • I have verified a documentation PR has been linked and is approved (or not applicable)
  • I have ran this PR locally and have tested the fix/feature
  • I have verified that stories have been added to storybook (or not applicable)
  • I have tested existing stories in storybook to verify no regression has occured
  • I have tested the solution in at least two browsers (Edge + 1 non-Chromium based browser)

Copy link
Contributor

@musale musale left a comment

Choose a reason for hiding this comment

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

Shall we remove the breadcrumbs story since we have the file-composite component? https://mgt.dev/next/?path=/story/components-mgt-file-list--open-folder-breadcrumbs

@gavinbarron
Copy link
Member Author

Shall we remove the breadcrumbs story since we have the file-composite component? https://mgt.dev/next/?path=/story/components-mgt-file-list--open-folder-breadcrumbs

No, I think we'll leave that as a standalone component. as folks can use it independently of the file composite. However, we should expose it as a React component too

@github-actions
Copy link

The updated storybook is available here

@gavinbarron gavinbarron mentioned this pull request Mar 11, 2023
6 tasks
@github-actions
Copy link

The updated storybook is available here

@gavinbarron
Copy link
Member Author

gavinbarron commented Mar 22, 2023

  • page-size doesn't seem supported by default. We should also make sure all props from mgt-file-list are supported.

It is.
In this PR the properties from file and file-list were refactored to abstract base classes to eliminate the need to re-declare them on file-list, file-grid and file-composite

If you set the page-size value to 5 you see only 5 items. However, there is an existing bug in the file-list caching where values page-size is not considered during caching, so when you change the page-size to 20 without changing the underlying query being made to something not cached there is a cache hit and you see only the 10 results fetched by default.

Bug added to track caching issue separately #2147

@github-actions
Copy link

The updated storybook is available here

@github-actions
Copy link

The updated storybook is available here

@github-actions
Copy link

Code Coverage

Package Line Rate Branch Rate Complexity Health
mgt-components.src.components.mgt-breadcrumb 100% 67% 0
mgt-components.src.components.mgt-person 50% 42% 0
mgt-components.src.components.mgt-theme-toggle 100% 100% 0
mgt-components.src.components.sub-components.mgt-flyout 28% 12% 0
mgt-components.src.components.sub-components.mgt-spinner 100% 100% 0
mgt-components.src.graph 18% 8% 0
mgt-components.src.styles 90% 57% 0
mgt-components.src.utils 36% 4% 0
mgt-element.src 42% 0% 0
mgt-element.src.components 40% 0% 0
mgt-element.src.mock 53% 45% 0
mgt-element.src.providers 38% 22% 0
mgt-element.src.utils 43% 24% 0
providers.mgt-msal-provider.src 46% 38% 0
Summary 38% (730 / 1944) 20% (229 / 1126) 0

@github-actions
Copy link

The updated storybook is available here

f => f.id,
f => html`
<div
class="${this.isSelected(f) ? 'file-row selected' : 'file-row'}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
class="${this.isSelected(f) ? 'file-row selected' : 'file-row'}"
class="${classMap({'file-row': true, selected: this.isSelected(f)})}"

@musale
Copy link
Contributor

musale commented Mar 29, 2023

When interacting with the first 3 menu items on a file, they appear on top of the dialog/modal that is supposed to be active.

image

@github-actions
Copy link

The updated storybook is available here

@github-actions
Copy link

The updated storybook is available here

@github-actions
Copy link

The updated storybook is available here

@github-actions
Copy link

github-actions bot commented May 9, 2023

The updated storybook is available here

@github-actions
Copy link

github-actions bot commented May 9, 2023

The updated storybook is available here

@gavinbarron gavinbarron deleted the branch v3-feature-branch June 27, 2023 16:00
@gavinbarron gavinbarron mentioned this pull request Jun 27, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants