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 details to Paper View #144

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
26 changes: 26 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
"prettier": "^3.3.2",
"prettier-plugin-svelte": "^3.3.3",
"svelte": "^5.16.0",
"svelte-autosize": "^1.1.5",
"svelte-check": "^4.1.4",
"tailwind-merge": "^2.6.0",
"tailwind-variants": "^0.3.0",
Expand Down
6 changes: 4 additions & 2 deletions src/lib/components/composites/PaperBookmarkButton.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,17 @@
import Tooltip from "./Tooltip.svelte";

interface Props {
paperId: number;
loadingPaperId: Promise<number>;
Copy link
Contributor

Choose a reason for hiding this comment

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

What's your reasoning for pulling that promise inside this component?
Couldn't this component be used as is when wrapped in svelte's {#await ...}?

This question arises multiple times throughout the code, one comment thread is enough though imo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used this because the component does not heavily rely on the asynchronous loading, i.e., we can just display the component and as soon as the paper is loaded the functionality works as well.
Therefore, wrapping the component in an await block seemed too much to me.

Where else did you notice this?

isBookmarkedDefault: boolean;
}

const { paperId, isBookmarkedDefault }: Props = $props();
const { loadingPaperId, isBookmarkedDefault }: Props = $props();

let isBookmarked = $state(isBookmarkedDefault);
let isHovered = $state(false);
const tooltipText = $derived(isBookmarked ? "Remove from Reading List" : "Add to Reading List");
let paperId: number | undefined = $state(undefined);
loadingPaperId.then((id) => (paperId = id)).catch(() => (paperId = undefined));

const onMouseEnter = () => (isHovered = true);
const onMouseLeave = () => (isHovered = false);
Expand Down
28 changes: 28 additions & 0 deletions src/lib/components/composites/input/AbstractToggleableInput.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<script lang="ts">
import type { HTMLInputAttributes } from "svelte/elements";
import type { WithElementRef } from "bits-ui";
import { maxHeight } from "./abstract-max-height-action";
import ToggleableInput from "./ToggleableInput.svelte";

type Props = WithElementRef<HTMLInputAttributes> & {
isEditable: boolean;
maxHeightActionProps?: { showButtonBar: boolean; showAdditionalInfos: boolean };
};

let {
isEditable = $bindable(false),
maxHeightActionProps,
value = $bindable(),
}: Props = $props();
</script>

<!--
@component
Same as ToggleableInput, but configured for the abstract textarea on the paper details card.
-->
<ToggleableInput
{isEditable}
inputAction={maxHeight}
inputActionProps={maxHeightActionProps}
{value}
/>
49 changes: 49 additions & 0 deletions src/lib/components/composites/input/ToggleableInput.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<script lang="ts">
import type { HTMLInputAttributes } from "svelte/elements";
import type { WithElementRef } from "bits-ui";
import { cn } from "$lib/utils/shadcn-helper";
import autosize from "svelte-autosize";
import type { Action } from "svelte/action";

type Props = WithElementRef<HTMLInputAttributes> & {
isEditable: boolean;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
inputAction?: Action<HTMLTextAreaElement, any>;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
inputActionProps?: any;
};

let {
isEditable = $bindable(false),
inputAction = () => {},
inputActionProps = {},
value = $bindable(),
class: className,
}: Props = $props();
</script>

<!--
@component
A textarea that can be toggled between read-only and editable mode.

The `inputAction` and `inputActionProps` props are used to pass an action to the textarea.
This is used in `AbstractToggleableInput` and probably won't be needed elsewhere.

Usage:
```svelte
<ToggleableInput {isEditable} {value} />
```
-->
<textarea
use:autosize
use:inputAction={inputActionProps}
Comment on lines +38 to +39
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think this workaround is necessary? The abstract's textarea could instead fill the remaining space. It would reduce complexity with the caveat of always filling the remaining space even when provided with a short text (see image below). Visually there wouldn't be too much of a difference with that one exception.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What did you use to produce that?

I tried plenty of approaches and none worked when the text overflows. The action was some sort of last resort.

class={cn(
"bg-background px-1.5 py-1 min-h-8 w-full resize-none text-default focus-visible:outline-none border",
isEditable ? "border-input rounded-md" : "border-transparent",
className,
)}
rows="1"
{value}
readonly={!isEditable}
data-testid="toggleable-input"
></textarea>
59 changes: 59 additions & 0 deletions src/lib/components/composites/input/abstract-max-height-action.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import type { Action } from "svelte/action";

/**
* This action has the sole purpose of preventing the textarea from growing outside of the card it is in.
* This is achieved by setting the max height of the textarea to the remaining space in the card.
*
* Previous attempts to find a dynamic solution to this problem have failed, so this action is a workaround by
* using hard coded padding values that might change in the future.
*/
export const maxHeight: Action<
HTMLTextAreaElement,
{ showButtonBar: boolean; showAdditionalInfos: boolean }
> = (node, { showButtonBar, showAdditionalInfos }) => {
setMaxHeight(node, showButtonBar, showAdditionalInfos);

// Autosize action sets overflow X to scroll, so we need to reset it
node.addEventListener("input", () => {
node.style.overflowX = "hidden";
});

window.addEventListener("resize", () => {
setMaxHeight(node, showButtonBar, showAdditionalInfos);
});

return {
update({ showButtonBar, showAdditionalInfos }) {
setMaxHeight(node, showButtonBar, showAdditionalInfos);
},
destroy() {
window.removeEventListener("resize", () => {
setMaxHeight(node, showButtonBar, showAdditionalInfos);
});
},
};
};

/**
* Sets the max height of the textarea element based on the padding and position of the element.
*
* @param node - the textarea element
* @param showButtonBar - whether the button bar is shown
* @param showAdditionalInfos - whether the additional infos are shown
*/
function setMaxHeight(
node: HTMLTextAreaElement,
showButtonBar: boolean,
showAdditionalInfos: boolean,
) {
// Calculate max height based on the window height
// node.getBoundingClientRect().top gives the top y position of the element i.e. the start of the element
// 113/53 is the sum of all the paddings the button bar and a border
const padding =
(showButtonBar ? 113 : 53) + // 40 Pixels of button bar + 20 pixels of gap
(showAdditionalInfos ? 5 : 0); // somehow there's a 5px change when the additional infos are shown
const maxHeight = window.innerHeight - node.getBoundingClientRect().top - padding;
node.style.maxHeight = `${maxHeight}px`;
node.style.overflowY = "auto";
node.style.overflowX = "hidden";
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
interface Props {
user: User;
backRef?: string | undefined;
paper: Paper | PaperSpec;
loadingPaper: Promise<Paper | PaperSpec>;
}

const { user, backRef, paper }: Props = $props();
const { user, backRef, loadingPaper }: Props = $props();
</script>

<NavigationBar {user} {backRef}>
<PaperInfo {paper} />
<PaperInfo {loadingPaper} />
</NavigationBar>
Original file line number Diff line number Diff line change
Expand Up @@ -6,39 +6,40 @@
type TabValue = (typeof tabs)[number]["value"];
interface Props {
user: User;
project: Project;
projectId: number;
loadingProject: Promise<Project>;
defaultTabValue: TabValue;
}

const { user, project, defaultTabValue }: Props = $props();
const { user, projectId, loadingProject, defaultTabValue }: Props = $props();
const tabs = [
{
value: "dashboard",
label: "Dashboard",
href: `/project/${project.id}/dashboard`,
href: `/project/${projectId}/dashboard`,
},
{
value: "papers",
label: "Papers",
href: `/project/${project.id}/papers`,
href: `/project/${projectId}/papers`,
},
{
value: "statistics",
label: "Statistics",
href: `/project/${project.id}/statistics`,
href: `/project/${projectId}/statistics`,
},
{
value: "settings",
label: "Settings",
href: `/project/${project.id}/settings/general`,
href: `/project/${projectId}/settings/general`,
},
] as const;
</script>

<SimpleNavigationBar
{user}
backRef="/"
title={project.name}
loadingTitle={loadingProject.then((project) => project.name)}
tabs={tabs as unknown as Tab[]}
{defaultTabValue}
/>
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,31 @@
import NavigationBar from "./NavigationBar.svelte";
import type { Tab } from "$lib/components/composites/navigation-bar/types";
import type { User } from "$lib/model/backend";
import Skeleton from "$lib/components/primitives/skeleton/skeleton.svelte";

interface Props {
user: User;
backRef?: string | undefined;
title: string;
loadingTitle: Promise<string>;
tabs?: Tab[] | undefined;
defaultTabValue?: (typeof tabs)[number]["value"] | undefined;
}

const { user, backRef = undefined, title, tabs = [], defaultTabValue = "" }: Props = $props();
const {
user,
backRef = undefined,
loadingTitle,
tabs = [],
defaultTabValue = "",
}: Props = $props();
</script>

<NavigationBar {user} {backRef} {tabs} {defaultTabValue}>
<h2 class="place-content-center truncate">{title}</h2>
{#await loadingTitle}
<Skeleton class="h-7 w-56 rounded-full" />
{:then title}
<h2 class="place-content-center truncate">{title}</h2>
{:catch}
<h2 class="place-content-center">Error</h2>
{/await}
</NavigationBar>
43 changes: 26 additions & 17 deletions src/lib/components/composites/paper-components/PaperInfo.svelte
Original file line number Diff line number Diff line change
@@ -1,27 +1,36 @@
<script lang="ts">
import { Skeleton } from "$lib/components/primitives/skeleton";
import type { Paper, PaperSpec } from "$lib/model/backend";
import { getNames } from "$lib/utils/common-helper";

interface Props {
paper: Paper | PaperSpec;
loadingPaper: Promise<Paper | PaperSpec>;
}

const { paper }: Props = $props();
const { loadingPaper }: Props = $props();
</script>

<div class="grid grid-flow-row gap-0">
<div class="flex flex-row gap-1 items-center truncate">
{#if "id" in paper}
<div class="w-fit text-default-sb-nc text-neutral-500">#{paper.id}</div>
{/if}
<h2 class="place-content-center truncate">{paper.title}</h2>
{#await loadingPaper}
<div class="grid grid-flow-row gap-1.5">
<Skeleton class="h-6 w-56 sm:w-80 md:w-[30rem] lg:w-[44rem] rounded-full" />
<Skeleton class="h-[1.125rem] w-28 sm:w-40 md:w-[15rem] lg:w-[22rem] rounded-full" />
</div>
<div class="flex flex-row items-center text-hint truncate">
{#if paper.authors.length > 0}
<span class="place-content-start truncate"
>{paper.authors.map((a) => `${a.firstName} ${a.lastName}`).join(", ")}</span
>
{:else}
<span class="italic">unknown authors</span>
{/if}
{:then paper}
<div class="grid grid-flow-row gap-0">
<div class="flex flex-row gap-1 items-center truncate">
{#if "id" in paper}
<div class="w-fit text-default-sb-nc text-neutral-500">#{paper.id}</div>
{/if}
Comment on lines +21 to +23
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason the id could be missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is used when the data is only a PaperSpec with no ID. This is the case on the paper create page.

<h2 class="place-content-center truncate">{paper.title}</h2>
</div>
<div class="flex flex-row items-center text-hint truncate">
{#if paper.authors.length > 0}
<span class="place-content-start truncate">{getNames(paper.authors)}</span>
{:else}
<span class="italic">unknown authors</span>
{/if}
</div>
</div>
</div>
{:catch}
<span class="text-error">Failed to load paper</span>
{/await}
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ Usage:
? `border-l-4 ${reviewDecisionColor[paper.reviewData?.finalDecision ?? 'unreviewed']}`
: ''} rounded-md px-3 py-1.5"
>
<PaperInfo {paper} />
<PaperInfo loadingPaper={Promise.resolve(paper)} />
</div>
{#if paper.reviewData !== undefined && showReviewStatus}
{#each paper.reviewData.reviews as review}
Expand Down
Loading
Loading