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

Document download and delete #16

Merged
merged 2 commits into from
Jan 22, 2024
Merged

Document download and delete #16

merged 2 commits into from
Jan 22, 2024

Conversation

kyle1morel
Copy link
Collaborator

Description

Implements document download and delete functionality.
Cleans up the Files tab a bit - made things a bit larger, removed the border surrounding the whole thing, added hover effects

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

Copy link

Coverage Report (Frontend)

Totals Coverage
Statements: 36.73% ( 184 / 501 )
Methods: 35.29% ( 36 / 102 )
Lines: 41.05% ( 117 / 285 )
Branches: 27.19% ( 31 / 114 )

Copy link

Coverage Report (Application)

Totals Coverage
Statements: 43.57% ( 61 / 140 )
Methods: 26.32% ( 5 / 19 )
Lines: 60.76% ( 48 / 79 )
Branches: 19.05% ( 8 / 42 )

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.

looks great. works great.

app/src/services/document.ts Show resolved Hide resolved
alt="document header"
src="@/assets/images/bcboxy.png"
class="document-image"
/>
Copy link
Contributor

@TimCsaky TimCsaky Jan 18, 2024

Choose a reason for hiding this comment

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

i would try moving this to the #content template. i believe it'll make aligning the whole thing easier.

</script>

<template>
<Card class="pt-2 pb-1 text-center">
Copy link
Contributor

Choose a reason for hiding this comment

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

fyi, you can combine top and bottom padding using class py-1.
i think youre ok to remove these as they may mess with the grid layout.

class="hover-hand hover-shadow"
@click="documentService.downloadDocument(document.documentId)"
@document:deleted="
(documentId) => (documents = documents.filter((x: Document) => x.documentId !== documentId))
Copy link
Contributor

Choose a reason for hiding this comment

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

This is pretty cool.
i thought you would have to remove the element from documents but i guess filtering works.
i did test this thoroughly in the browser and it all looks great

</template>

<style lang="scss">
.document-image {
Copy link
Contributor

Choose a reason for hiding this comment

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

when it comes to showing the file type icon (eg: document, image, pdf or whatever) i dont think this layout will really work.. but we'll see.

@TimCsaky TimCsaky merged commit 520a282 into master Jan 22, 2024
19 of 20 checks passed
@kyle1morel kyle1morel deleted the feature/doc-download branch January 22, 2024 20:53
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.

3 participants