fix: added support for timePrecision by techbhavin · Pull Request #9423 · appsmithorg/appsmith

techbhavin · 2021-11-29T06:41:43Z

Description

Allow users to insert hour:time: seconds in date-time-picker widget

Fixes #9100

Type of change

New feature (non-breaking change which adds functionality)

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented on my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

Test coverage results 🧪

🟢 Total coverage has increased

// Code coverage diff between base branch:release and head branch: feature/improved-datetimepicker

Status	File	% Stmts	% Branch	% Funcs	% Lines
🟢	total	55.23 (0.01)	37.02 (0.01)	34.08 (0.01)	55.75 (0.02)
🟢	app/client/src/selectors/commentsSelectors.ts	85.25 (1.64)	64.71 (2.95)	73.33 (0)	90.59 (2.35)
✨ 🆕	app/client/src/widgets/DatePickerWidget2/constants.ts	100	100	100	100
🔴	app/client/src/widgets/DatePickerWidget2/component/index.tsx	27.78 (0)	3.08 (-0.09)	6.25 (0)	27.14 (0)
🟢	app/client/src/widgets/DatePickerWidget2/widget/index.tsx	84.62 (0.62)	0 (0)	75 (0)	87.5 (0.54)
🟢	app/client/src/widgets/TableWidget/component/CascadeFields.tsx	28.68 (0.55)	24.66 (0)	3.13 (0)	30.33 (0.58)

techbhavin · 2021-11-30T05:55:42Z

/ok-to-test sha=c781f1f

github-actions · 2021-11-30T05:56:15Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1519427752.
Workflow: Appsmith External Integration Test Workflow.
Commit: c781f1f.
PR: 9423.

shwetha-ramesh · 2021-12-02T09:25:17Z

@techbhavin tested this PR

the time slot input box appears only when the input is dropped on the canvas. When the user changes from the default format to any other format and moved back to the default format the time slot input disappears. (refer video)
Time slot input not available for any of the format that.

https://loom.com/share/406044fb99a4403489afcc487dd83ed2

techbhavin · 2021-12-02T13:08:47Z

/ok-to-test sha=8fbf0d4

github-actions · 2021-12-02T13:09:23Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1530702978.
Workflow: Appsmith External Integration Test Workflow.
Commit: 8fbf0d4.
PR: 9423.

techbhavin · 2021-12-07T03:46:53Z

/ok-to-test sha=e9bc116

github-actions · 2021-12-07T03:47:29Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1547780745.
Workflow: Appsmith External Integration Test Workflow.
Commit: e9bc116.
PR: 9423.

shwetha-ramesh · 2021-12-07T09:59:23Z

@techbhavin tested this PR. Below are the points.

The seconds field must not be shown for YYYY-MM-DD HH:mm format or any format that does not have ss mentioned, irrespective of what 'time precision' is set
https://loom.com/share/0f9adc6e148d42fda0c96a5a7842c068
Clear must clear the timeslot values also.

https://loom.com/share/b10f5565f2b74c7ea760de1d7db4c492

Since the Time precision field is placed at the end of General settings, the user might not know a field as such exists and when changes the format which has hh:mm:ss and not be able to change the seconds field.

techbhavin · 2021-12-07T12:37:08Z

The seconds field must not be shown for YYYY-MM-DD HH:mm format or any format that does not have ss mentioned, irrespective of what 'time precision' is set

In a current implementation, we show hh:mm in every format. did we want to do as per @shwetha-ramesh mention?

ALSO NEED FEEDBACK about point 2 , 3

@somangshu @sbalaji1192 @riodeuno @Tooluloope

sbalaji1192 · 2021-12-09T10:00:21Z

@techbhavin @shwetha-ramesh @Tooluloope
WRT,

Current implementation seems good as it's not leading to any bug. Having two configs controlling the same field is bit much to maintain.
Agreed on the callout. we need to clear the time as well. since closing and opening the popup resets the time.
Let's move the time precision right below to the time format.

…appsmithorg/appsmith into feature/improved-datetimepicker

techbhavin · 2021-12-10T07:48:56Z

/ok-to-test sha=e0c5d8a

github-actions · 2021-12-10T07:49:27Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1562431314.
Workflow: Appsmith External Integration Test Workflow.
Commit: e0c5d8a.
PR: 9423.

techbhavin · 2021-12-10T07:54:12Z

@sbalaji1192 @shwetha-ramesh @Tooluloope

point 3: updated
point 2: here we have a limitation in the library we can’t reset the time picker value.

github-actions · 2021-12-17T12:50:56Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1592014924.
Workflow: Appsmith External Integration Test Workflow.
Commit: db2e9ec.
PR: 9423.

shwetha-ramesh · 2021-12-21T07:20:02Z

As agreed upon with above comments. Moving to DONE.

vercel · 2021-12-21T09:15:56Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/get-appsmith/appsmith/7oTMNYZZ6q4UfusTTxw8iXiK5KDk
✅ Preview: https://appsmith-git-feature-improved-datetimepicker-get-appsmith.vercel.app

techbhavin · 2021-12-21T09:17:40Z

/ok-to-test sha=705c400

github-actions · 2021-12-21T09:18:17Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1606057955.
Workflow: Appsmith External Integration Test Workflow.
Commit: 705c400.
PR: 9423.

github-actions · 2021-12-21T11:53:40Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1606057955.
Workflow: Appsmith External Integration Test Workflow.
Commit: 705c400.
PR: 9423.

techbhavin · 2021-12-21T12:50:09Z

/ok-to-test sha=1fa162c

github-actions · 2021-12-21T13:08:50Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1606898439.
Workflow: Appsmith External Integration Test Workflow.
Commit: 1fa162c.
PR: 9423.

techbhavin · 2021-12-22T06:34:27Z

/ok-to-test sha=1fa162c

github-actions · 2021-12-22T06:35:00Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1610141047.
Workflow: Appsmith External Integration Test Workflow.
Commit: 1fa162c.
PR: 9423.

github-actions · 2021-12-22T10:51:49Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1610141047.
Workflow: Appsmith External Integration Test Workflow.
Commit: 1fa162c.
PR: 9423.

sbalaji1192 · 2021-12-22T17:07:57Z

app/client/src/widgets/DatePickerWidget2/index.ts

 import IconSVG from "./icon.svg";
 import moment from "moment";
 import { GRID_DENSITY_MIGRATION_V1 } from "widgets/constants";
+import { TimePrecision } from "@blueprintjs/datetime";


Lets use our TimePrecision

techbhavin · 2021-12-23T06:58:07Z

/ok-to-test sha=bd8d93a

github-actions · 2021-12-23T06:58:42Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/1614512734.
Workflow: Appsmith External Integration Test Workflow.
Commit: bd8d93a.
PR: 9423.

fix: 9100 added support for timePrecision

c781f1f

techbhavin added Widgets Product This label groups issues related to widgets UI Building Pod labels Nov 29, 2021

techbhavin requested a review from jsartisan November 29, 2021 06:41

github-actions bot added the Bug Something isn't working label Nov 29, 2021

techbhavin changed the title ~~fix:added support for timePrecision~~ fix: added support for timePrecision Nov 30, 2021

jsartisan previously approved these changes Nov 30, 2021

View reviewed changes

timePrecision default value added

8fbf0d4

techbhavin dismissed jsartisan’s stale review via 8fbf0d4 December 2, 2021 12:29

techbhavin requested a review from jsartisan December 2, 2021 13:08

Merge branch 'release' into feature/improved-datetimepicker

e9bc116

jsartisan previously approved these changes Dec 6, 2021

View reviewed changes

techbhavin added 3 commits December 10, 2021 12:44

fix : Time precision position update

a3e79cf

Merge branch 'release' into feature/improved-datetimepicker

f5b4358

Merge branch 'feature/improved-datetimepicker' of https://github.com/…

e0c5d8a

…appsmithorg/appsmith into feature/improved-datetimepicker

techbhavin dismissed jsartisan’s stale review via e0c5d8a December 10, 2021 07:16

techbhavin requested a review from jsartisan December 10, 2021 07:48

Merge branch 'release' into feature/improved-datetimepicker

705c400

vercel bot deployed to Preview December 21, 2021 09:15 View deployment

Merge branch 'release' into feature/improved-datetimepicker

1fa162c

vercel bot deployed to Preview December 21, 2021 12:38 View deployment

sbalaji1192 reviewed Dec 22, 2021

View reviewed changes

techbhavin added 2 commits December 23, 2021 12:25

fix : import update

76ec037

Merge branch 'release' into feature/improved-datetimepicker

bd8d93a

vercel bot deployed to Preview December 23, 2021 06:57 View deployment

sbalaji1192 approved these changes Dec 23, 2021

View reviewed changes

techbhavin enabled auto-merge (squash) December 23, 2021 10:00

sbalaji1192 disabled auto-merge December 23, 2021 11:44

techbhavin enabled auto-merge (squash) December 23, 2021 12:07

techbhavin merged commit 244b089 into release Dec 23, 2021

techbhavin deleted the feature/improved-datetimepicker branch December 23, 2021 14:17

somangshu mentioned this pull request Jan 5, 2022

[Feature] Only Date option #3516

Closed

dilippitchika added the Documentation Improvements or additions to documentation label Jan 5, 2022

leotom2000 pushed a commit that referenced this pull request Jan 6, 2022

fix: added support for timePrecision (#9423)

2f0cbc4

Conversation

techbhavin commented Nov 29, 2021 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Checklist:

Test coverage results 🧪

Uh oh!

techbhavin commented Nov 30, 2021

Uh oh!

github-actions bot commented Nov 30, 2021

Uh oh!

shwetha-ramesh commented Dec 2, 2021

Uh oh!

techbhavin commented Dec 2, 2021

Uh oh!

github-actions bot commented Dec 2, 2021

Uh oh!

techbhavin commented Dec 7, 2021

Uh oh!

github-actions bot commented Dec 7, 2021

Uh oh!

shwetha-ramesh commented Dec 7, 2021

Uh oh!

techbhavin commented Dec 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbalaji1192 commented Dec 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

techbhavin commented Dec 10, 2021

Uh oh!

github-actions bot commented Dec 10, 2021

Uh oh!

techbhavin commented Dec 10, 2021

Uh oh!

github-actions bot commented Dec 17, 2021

Uh oh!

shwetha-ramesh commented Dec 21, 2021

Uh oh!

vercel bot commented Dec 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

techbhavin commented Dec 21, 2021

Uh oh!

github-actions bot commented Dec 21, 2021

Uh oh!

github-actions bot commented Dec 21, 2021

Uh oh!

techbhavin commented Dec 21, 2021

Uh oh!

github-actions bot commented Dec 21, 2021

Uh oh!

techbhavin commented Dec 22, 2021

Uh oh!

github-actions bot commented Dec 22, 2021

Uh oh!

github-actions bot commented Dec 22, 2021

Uh oh!

sbalaji1192 Dec 22, 2021

Choose a reason for hiding this comment

Uh oh!

techbhavin commented Dec 23, 2021

Uh oh!

github-actions bot commented Dec 23, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

techbhavin commented Nov 29, 2021 •

edited by github-actions bot

Loading

techbhavin commented Dec 7, 2021 •

edited

Loading

sbalaji1192 commented Dec 9, 2021 •

edited

Loading

vercel bot commented Dec 21, 2021 •

edited

Loading