Skip to content

Commit

Permalink
Fix: ui.datetime() rendering with empty initial value (#3249)
Browse files Browse the repository at this point in the history
Fixes #3246

- Added test to verify datetime picker renders with empty initial value
- Fixed DateTimePickerPlugin to handle undefined values properly

Link to Devin run:
https://app.devin.ai/sessions/47bc25251a1c4f8185891042b4964ebf

Co-authored-by: Myles Scolnick <myles@marimo.io>
  • Loading branch information
devin-ai-integration[bot] and mscolnick authored Dec 20, 2024
1 parent e66f93b commit 3b9cba1
Show file tree
Hide file tree
Showing 10 changed files with 80 additions and 25 deletions.
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* Copyright 2024 Marimo. All rights reserved. */
import { render } from "@testing-library/react";
import { DataTable } from "../data-table";
import { describe, expect, it } from "vitest";
Expand Down
49 changes: 25 additions & 24 deletions frontend/src/css/md.css
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,8 @@ button .prose.prose {

.prose .critic {
font-family: inherit;
-webkit-border-radius: 3px;
-moz-border-radius: 3px;
border-radius: 3px;
border-radius: 3px;
border-radius: 3px;
border-style: solid;
border-width: 1px;
Expand All @@ -188,18 +188,19 @@ button .prose.prose {
text-decoration: none;
}

.prose .critic:before,
.prose .critic:after {
.prose .critic::before,
.prose .critic::after {
content: "\00a0";
padding-top: 0.1rem;
padding-bottom: 0.1rem;
font-size: initial;
}

.prose .block:before,
.prose .block:after {
.prose .block::before,
.prose .block::after {
content: "";
}

.prose mark.critic {
border-color: var(--orange-9);
background: var(--orange-3);
Expand All @@ -221,11 +222,11 @@ button .prose.prose {
border: none;
}

.prose ins.break:before,
.prose del.break:before {
.prose ins.break::before,
.prose del.break::before {
content: "\00a0\b6\00a0";
-webkit-border-radius: 3px;
-moz-border-radius: 3px;
border-radius: 3px;
border-radius: 3px;
border-radius: 3px;
}

Expand All @@ -234,13 +235,13 @@ button .prose.prose {
content: "";
}

.prose ins.break:before {
.prose ins.break::before {
color: var(--green-9);
border: 1px solid var(--green-9);
background: var(--green-3);
}

.prose del.break:before {
.prose del.break::before {
color: var(--red-9);
border: 1px solid var(--red-9);
background: var(--red-3);
Expand All @@ -253,32 +254,32 @@ button .prose.prose {
border-bottom: 1px solid var(--blue-9);
}

.prose span.critic:before,
.prose span.critic:after {
.prose span.critic::before,
.prose span.critic::after {
font-size: inherit;
background: var(--blue-3);
border: 1px solid var(--blue-9);
}

.prose span.critic:before {
.prose span.critic::before {
content: "\00a0\bb";
border-right: none;
-webkit-border-top-left-radius: 3px;
-moz-border-top-left-radius: 3px;
border-top-left-radius: 3px;
-webkit-border-bottom-left-radius: 3px;
-moz-border-bottom-left-radius: 3px;
border-top-left-radius: 3px;
border-top-left-radius: 3px;
border-bottom-left-radius: 3px;
border-bottom-left-radius: 3px;
border-bottom-left-radius: 3px;
}

.prose span.critic:after {
.prose span.critic::after {
content: "\ab\00a0";
border-left: none;
-webkit-border-top-right-radius: 3px;
-moz-border-top-right-radius: 3px;
border-top-right-radius: 3px;
-webkit-border-bottom-right-radius: 3px;
-moz-border-bottom-right-radius: 3px;
border-top-right-radius: 3px;
border-top-right-radius: 3px;
border-bottom-right-radius: 3px;
border-bottom-right-radius: 3px;
border-bottom-right-radius: 3px;
}

Expand Down
1 change: 1 addition & 0 deletions frontend/src/hooks/__tests__/useBoolean.test.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* Copyright 2024 Marimo. All rights reserved. */
import { expect, describe, it, vi } from "vitest";
import { renderHook, act } from "@testing-library/react";
import { useBoolean } from "../useBoolean";
Expand Down
1 change: 1 addition & 0 deletions frontend/src/hooks/__tests__/useDebounce.test.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* Copyright 2024 Marimo. All rights reserved. */
import { expect, describe, it, vi, beforeEach, afterEach } from "vitest";
import { renderHook, act } from "@testing-library/react";
import { useDebounce, useDebounceControlledState } from "../useDebounce";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* Copyright 2024 Marimo. All rights reserved. */
import { renderHook } from "@testing-library/react";
import { describe, expect, it, vi } from "vitest";
import { useEffectSkipFirstRender } from "../useEffectSkipFirstRender";
Expand Down
1 change: 1 addition & 0 deletions frontend/src/hooks/__tests__/useInterval.test.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* Copyright 2024 Marimo. All rights reserved. */
import { expect, describe, it, vi, beforeEach, afterEach } from "vitest";
import { renderHook } from "@testing-library/react";
import { useInterval } from "../useInterval";
Expand Down
1 change: 1 addition & 0 deletions frontend/src/hooks/__tests__/useTimer.test.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* Copyright 2024 Marimo. All rights reserved. */
import { expect, describe, it, vi, beforeEach, afterEach } from "vitest";
import { renderHook, act } from "@testing-library/react";
import { useTimer } from "../useTimer";
Expand Down
1 change: 1 addition & 0 deletions frontend/src/hooks/useEffectSkipFirstRender.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* Copyright 2024 Marimo. All rights reserved. */
import { useEffect, useRef } from "react";

/**
Expand Down
5 changes: 4 additions & 1 deletion frontend/src/plugins/impl/DateTimePickerPlugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,14 @@ const DateTimePickerComponent = (props: DateTimePickerProps): JSX.Element => {
props.setValue(isoStr);
};

// Add null check and default to undefined when no value is provided
const parsedValue = props.value ? parseDateTime(props.value) : undefined;

return (
<Labeled label={props.label} fullWidth={props.fullWidth}>
<DatePicker
granularity="minute"
value={parseDateTime(props.value)}
value={parsedValue}
onChange={handleInput}
aria-label={props.label ?? "date time picker"}
minValue={parseDateTime(props.start)}
Expand Down
44 changes: 44 additions & 0 deletions frontend/src/plugins/impl/__tests__/DateTimePickerPlugin.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/* Copyright 2024 Marimo. All rights reserved. */
import { DateTimePickerPlugin } from "../DateTimePickerPlugin";
import { render } from "@testing-library/react";
import { expect, describe, it } from "vitest";
import type { IPluginProps } from "../../types";

interface DateTimeData {
label: string | null;
start: string;
stop: string;
fullWidth: boolean;
}

describe("DateTimePickerPlugin", () => {
it("should render when initial value is not provided", () => {
const plugin = new DateTimePickerPlugin();
// Create a host element as required by IPluginProps
const host = document.createElement("div");
const props: IPluginProps<string, DateTimeData> = {
host,
value: "", // Empty string instead of undefined since type T = string
setValue: (valueOrFn) => {
// No-op function to satisfy lint requirements
if (typeof valueOrFn === "function") {
valueOrFn("");
}
},
data: {
label: null,
start: "2024-01-01T00:00:00",
stop: "2024-12-31T23:59:59",
fullWidth: false,
},
functions: {},
};
const { container } = render(plugin.render(props));

// Check if the component renders at all
expect(container.innerHTML).not.toBe("");
// Check for the date picker group
const datePicker = container.querySelector('[class*="group"]');
expect(datePicker).not.toBeNull();
});
});

0 comments on commit 3b9cba1

Please sign in to comment.