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.

Sign up for GitHub

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

Jump to bottom

Feat/2941 file properties #31

Merged
merged 15 commits into from
Feb 2, 2023
Merged

Feat/2941 file properties #31

merged 15 commits into from
Feb 2, 2023

Conversation

bcgv123
Copy link
Contributor

@bcgv123 bcgv123 commented Jan 27, 2023

Description

Added single page for file properties, refined partial file properties view (added metadata, tags). New route added to support single page.

Types of changes

Checklist

  • I have read the CONTRIBUTING doc
  • I have checked that unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

kamorel
kamorel reviewed Jan 30, 2023
View reviewed changes
Copy link
Contributor

@kamorel kamorel left a comment

Choose a reason for hiding this comment

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

Review screens against UI mockups. The sidebar is currently displaying the same contents as the full screen view, but the mockups do not reflect this.

jujaga
jujaga reviewed Jan 30, 2023
View reviewed changes
Copy link
Member

@jujaga jujaga left a comment

Choose a reason for hiding this comment

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

Make sure to do a full proper rebase on this branch before we proceed further.

frontend/src/components/object/ObjectProperties.vue Outdated Show resolved Hide resolved
frontend/src/store/objectStore.ts Outdated Show resolved Hide resolved
TimCsaky
TimCsaky reviewed Jan 31, 2023
View reviewed changes
Copy link
Contributor

@TimCsaky TimCsaky left a comment

Choose a reason for hiding this comment

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

just took a quick look over. Looking good! not much to add at this stage

frontend/src/components/object/ObjectAccess.vue Outdated Show resolved Hide resolved
frontend/src/components/object/ObjectFileDetails.vue Outdated Show resolved Hide resolved
frontend/src/services/objectService.ts Outdated Show resolved Hide resolved
frontend/src/store/objectStore.ts Outdated Show resolved Hide resolved
frontend/src/store/objectStore.ts Outdated Show resolved Hide resolved
@bcgv123 bcgv123 force-pushed the feat/2941-file-properties branch from 407f8c5 to be92269 Compare January 31, 2023 18:02
@github-actions
Copy link

Coverage Report (Application)

Totals Coverage
Statements: 75% ( 51 / 68 )
Methods: 62.5% ( 5 / 8 )
Lines: 82.61% ( 38 / 46 )
Branches: 57.14% ( 8 / 14 )

@github-actions
Copy link

Coverage Report (Frontend)

Totals Coverage
Statements: 43.68% ( 38 / 87 )
Methods: 25% ( 6 / 24 )
Lines: 53.85% ( 28 / 52 )
Branches: 36.36% ( 4 / 11 )

@bcgv123 bcgv123 force-pushed the feat/2941-file-properties branch 2 times, most recently from 5666e83 to 8f7dba0 Compare January 31, 2023 18:23
jujaga
jujaga previously requested changes Feb 2, 2023
View reviewed changes
Copy link
Member

@jujaga jujaga left a comment

Choose a reason for hiding this comment

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

Please make sure the commit history lineage is linear without unnecessary merge commits before we fold this in.

@bcgv123 bcgv123 force-pushed the feat/2941-file-properties branch from f9fe66e to 52a7521 Compare February 2, 2023 19:34
@jujaga jujaga dismissed their stale review February 2, 2023 19:36

Resolved

@jujaga jujaga merged commit 0c31d66 into master Feb 2, 2023
@jujaga jujaga deleted the feat/2941-file-properties branch February 2, 2023 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TimCsaky TimCsaky TimCsaky approved these changes

@kamorel kamorel kamorel approved these changes

@jujaga jujaga jujaga approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bcgv123 @jujaga @kamorel @TimCsaky