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

Added paper view #89

Open
wants to merge 24 commits into
base: develop
Choose a base branch
from
Open

Conversation

Slartibartfass2
Copy link
Contributor

@Slartibartfass2 Slartibartfass2 commented Nov 19, 2024

Closes #34

What I have made

  • the root layout file has now a container with the style that is used in every page of our application
  • added a reusable tooltip component
  • added some aria labels to existing components for improved accessibility
  • added paper view, consisting of the following components
    • bookmark button (logic is implemented in [Use Case]: Add Paper To Reading List #99 and [Use Case]: Remove Paper From Reading List #100)
    • navigation button = previous/next paper button (due to the tooltip being unable to be an a tag when providing a href prop (like a button would do), this button uses goto to navigate to the previous/next paper)
    • paper cards are the two card components on the paper view, which display paper information
    • the content of these cards will be implemented in other issues, this also includes the testing for these cards, testing it now wouldn't make sense
    • decision buttons to either accept, decline or mark a paper as undecided (maybe button)
      • there is a component for each decision, which seems a lot, but providing different styles depending on a type/state prop was less readable
  • added a setup test file to mock the goto method used in the navigation buttons
  • the paper view component is shown on the project dependent and independent pages
  • this PR adds the general structure and the views will be extended in future issues, I wouldn't add more to it now

Checklist

Either tick or cross out the items that do not apply (using ~~example text~~) and give a reason why the item does not apply.

  • I have updated the documentation accordingly and commented my code
  • I have added tests that prove my fix is effective or that my feature works
    • [ ] unit tests (no logic outside of minor component logic was added)
    • integration tests
    • [ ] end-to-end tests (we do not have those yet)
  • (for reviewer) I have checked the implementation against the requirements

@Slartibartfass2 Slartibartfass2 self-assigned this Nov 19, 2024
@Slartibartfass2 Slartibartfass2 linked an issue Nov 19, 2024 that may be closed by this pull request
Copy link

github-actions bot commented Nov 19, 2024

LCOV of commit 11fe5f0 during Code Quality Checks #376

Summary coverage rate:
  lines......: 64.1% (446 of 696 lines)
  functions..: 18.3% (15 of 82 functions)
  branches...: no data found

Files changed coverage rate:
                                                                                                  |Lines       |Functions  |Branches    
  Filename                                                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================================================
  src/lib/components/composites/PaperBookmarkButton.svelte                                        | 100%     27|50.0%     4|    -      0
  src/lib/components/composites/Tooltip.svelte                                                    | 100%      4| 0.0%     1|    -      0
  src/lib/components/composites/navigation-bar/NavigationBar.svelte                               | 100%     14| 0.0%     1|    -      0
  src/lib/components/composites/navigation-bar/PaperNavigationBar.svelte                          | 100%      1| 0.0%     1|    -      0
  src/lib/components/composites/navigation-bar/SimpleNavigationBar.svelte                         | 100%      2| 0.0%     1|    -      0
  src/lib/components/composites/paper-components/paper-view/PaperNavigationButton.svelte          | 100%      6| 0.0%     1|    -      0
  src/lib/components/composites/paper-components/paper-view/PaperView.svelte                      | 100%      8|50.0%     2|    -      0
  src/lib/components/composites/paper-components/paper-view/cards/PaperCard.svelte                | 100%      2| 0.0%     1|    -      0
  src/lib/components/composites/paper-components/paper-view/cards/PaperCardContent.svelte         | 100%      1| 0.0%     1|    -      0
  src/lib/components/composites/paper-components/paper-view/cards/PaperDetailsCard.svelte         | 100%      4| 0.0%     1|    -      0
  src/lib/components/composites/paper-components/paper-view/cards/PaperResearchContextCard.svelte | 100%      4| 0.0%     1|    -      0
  src/lib/components/composites/paper-components/paper-view/decision-buttons/AcceptButton.svelte  | 100%      4| 0.0%     1|    -      0
  src/lib/components/composites/paper-components/paper-view/decision-buttons/DecisionButton.svelte| 100%      4|50.0%     2|    -      0
  src/lib/components/composites/paper-components/paper-view/decision-buttons/DeclineButton.svelte | 100%      4| 0.0%     1|    -      0
  src/lib/components/composites/paper-components/paper-view/decision-buttons/MaybeButton.svelte   | 100%      4| 0.0%     1|    -      0
  src/lib/components/composites/tabs/UnderlineTabsList.svelte                                     | 100%      4| 0.0%     1|    -      0
  src/lib/components/primitives/button/button.svelte                                              | 100%     33| 0.0%     1|    -      0
  src/lib/components/primitives/button/index.ts                                                   | 100%      1| 0.0%     1|    -      0
  src/lib/components/primitives/tooltip/index.ts                                                  | 100%      4| 0.0%     1|    -      0
  src/lib/components/primitives/tooltip/tooltip-content.svelte                                    | 100%      7| 0.0%     1|    -      0

@Slartibartfass2 Slartibartfass2 force-pushed the feat/34-use-case-open-paper-view branch 9 times, most recently from 4de9f9f to 895dbe0 Compare November 25, 2024 17:05
@Slartibartfass2 Slartibartfass2 force-pushed the feat/34-use-case-open-paper-view branch 4 times, most recently from af5e97c to 59f9851 Compare November 28, 2024 16:18
@Slartibartfass2 Slartibartfass2 marked this pull request as ready for review November 28, 2024 16:19
@Slartibartfass2 Slartibartfass2 force-pushed the feat/34-use-case-open-paper-view branch from 59f9851 to 5139521 Compare December 2, 2024 08:54
@Slartibartfass2 Slartibartfass2 removed the request for review from luca-schlecker December 2, 2024 09:00
@Slartibartfass2 Slartibartfass2 marked this pull request as draft December 2, 2024 09:00
@Slartibartfass2 Slartibartfass2 force-pushed the feat/34-use-case-open-paper-view branch from 5139521 to fc5ddfe Compare December 2, 2024 22:41
@Slartibartfass2 Slartibartfass2 marked this pull request as ready for review December 3, 2024 21:32
@Slartibartfass2 Slartibartfass2 force-pushed the feat/34-use-case-open-paper-view branch from 6fabcf3 to 3360d88 Compare December 5, 2024 07:08
@Merseleo Merseleo removed their request for review December 5, 2024 12:43
@Slartibartfass2 Slartibartfass2 force-pushed the feat/34-use-case-open-paper-view branch 2 times, most recently from c019ecd to f2c89ac Compare December 6, 2024 17:09
Copy link
Contributor

@luca-schlecker luca-schlecker left a comment

Choose a reason for hiding this comment

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

Great work! There were some things though that stood out to me. Looking forward to hearing your thoughts on these points.

Copy link
Contributor Author

@Slartibartfass2 Slartibartfass2 left a comment

Choose a reason for hiding this comment

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

Thanks for the review, I'll rebase when we're finished so that you see the changes I made.

@Slartibartfass2 Slartibartfass2 force-pushed the feat/34-use-case-open-paper-view branch from 68832c9 to 8f85015 Compare December 16, 2024 09:21
@Slartibartfass2 Slartibartfass2 force-pushed the feat/34-use-case-open-paper-view branch from 8f85015 to 11fe5f0 Compare December 17, 2024 16:23
Copy link
Contributor

@luca-schlecker luca-schlecker left a comment

Choose a reason for hiding this comment

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

LGTM. 👍

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.

[Use Case]: Open Paper View
2 participants