Added date_input_widget #67

heejit · 2023-06-25T15:56:44Z

No description provided.

Jacalz · 2023-06-25T16:04:21Z

Can you please stick to one item per PR? I don’t we should land separate widgets and layouts within the same PR.

Is the HBox layout not kind of a duplicate of #64?

heejit · 2023-06-25T17:26:57Z

I wanted to layout widget horizontally and can't find any other layout if it is same then reject it. One difference is this layout only expand horizontally.

heejit · 2023-06-25T17:29:08Z

I will stick to one item per PR for now on.

Jacalz · 2023-06-25T18:29:03Z

In that case, can you please update the PR (or open a new one id you prefer) that only includes one of the features?

I do think that the layout in question indeed is mostly the same as the one linked above. I'll try it complete that PR some time in the future. The full list of layouts in Fyne can be found here: https://developer.fyne.io/explore/layouts

heejit · 2023-06-25T18:32:25Z

okay let me just add the date input widget

heejit · 2023-06-25T18:38:57Z

I have remove the layout code.

Jacalz · 2023-06-25T19:05:34Z

Thanks. I'll try to review the code some day going forward

andydotxyz

I just had a quick glance over and a few things stood out.
Also can you add an item to the README with an image to show it in action please?

andydotxyz · 2023-06-26T08:47:30Z

widget/widget_date_input.go

+
+const ConstDefaultDateFormat = "02-Jan-2006"
+
+var DayPos = []int{0, 2}


Should these fields be part of the public API?

andydotxyz · 2023-06-26T08:47:45Z

widget/widget_date_input.go

+var MonthPos = []int{3, 6}
+var YearPos = []int{7, 11}
+
+type MyDateEntry struct {


I think naming here could be improved

andydotxyz · 2023-06-26T08:48:31Z

widget/widget_date_input.go

+// we assume 1st part is always Day
+// input = 1 -> 1-CurMonth-CurYear
+// input = 1.5, 1/5, 1-5 -> 1-5-CurYear
+func (e *MyDateEntry) parse_and_update_date() {


Try to match the Go style for naming - your other methods are formatted correctly. parseAndUpdateData would be more in-keeping

I will update naming as per go naming rule

andydotxyz · 2023-10-04T12:31:22Z

I just came back to this (sorry for the delay) and notice that MyDateEntry is still a fairly strange name ;).
Also if you can rebase onto latest main the tests should all pass again.

heejit · 2023-10-05T06:31:50Z

I just came back to this (sorry for the delay) and notice that MyDateEntry is still a fairly strange name ;). Also if you can rebase onto latest main the tests should all pass again.

I have rewrite the whole widget this time using /richtext : https://github.com/heejit/fyne_codes/blob/main/widget_date_input.go

Jacalz

Thanks for the update. I just did a very quick code inspection on my phone and probably missed some things but I hope the review is helpful either way.

Jacalz · 2023-10-05T20:22:07Z

widget/widget_date_input.go

+var _ fyne.Tappable = (*JDateInputWidget)(nil)
+
+// Custom Date Input Widget using RichText
+type JDateInputWidget struct {


Someone using this will get widget.JDateInputWidget which stutters. No need to say "widget" again. Also, what is the "J" for?

The README mentions DateEntry so why not call it that (or DateInput)?

I want to prefix my widget names with J no special meaning

We need our APIs to be consistent and be easy to understand. I would suggest DateInput.

Jacalz · 2023-10-05T20:23:04Z

widget/widget_date_input.go

+}
+
+func (t *JDateInputWidget) TypedRune(k rune) {
+ // needed


Not needed? I know it is needed for the interface but the comment makes that unclear. If you don't need this method for anything other than satisfying the interface, you can just remove the comment

Jacalz · 2023-10-05T20:25:05Z

widget/widget_date_input.go

+
+// End: implement focusable interface
+
+// Start: implemnet Tappable


Should be "implement". This comment applies to more places

Jacalz · 2023-10-05T20:27:52Z

widget/widget_date_input.go

+}
+
+// constructor
+func NewJDateInputWidget(app fyne.App, window fyne.Window) *JDateInputWidget {


All exported functions and methods should have a comment describing if them. See already existing code in this repo for examples.

The comments also need to follow the Go standard starting with the name of the function.

Add comments

andydotxyz · 2024-09-07T17:31:14Z

I just wanted to see the status of this and tried to resolve the conflict but as it was a PR from your master branch GitHub warned me not to, so I didn't - but it's a simple change.

I am sorry it took so long to get back to this - I hadn't noticed that there were commits since the last feedback.
However the naming is still not resolved as requested by Jacalz and there are references to your fork in the README.

Also it seems like you're looking to behave like an Entry but it isn't an Entry - so why not just extend Entry?
I have been working on an alternative picker based on the Calendar here and a simple Entry widget with an accessory for popping up the calendar, changes to the Entry theming made this fairly simple - not sure if this changes your thoughts?

heejit · 2024-09-08T06:44:17Z

I wanted dateinputwidget to used via keyboard(arrow keys) that is why designed like that and if you already working on it then there is no need of this again.

andydotxyz · 2024-09-24T13:27:57Z

The keyboard input could be a great extension of what has landed in fyne - if you would consider basing such a feature on top of the new widget that would be amazing, thanks.
However closing this PR so we can focus over there now.

heejit · 2024-09-24T13:40:19Z

okay

Added date_input_widget and hbox_ratio_layout

f51440a

Remove the Layout

8a78c0a

andydotxyz changed the title ~~Added date_input_widget and hbox_ratio_layout~~ Added date_input_widget Jun 26, 2023

andydotxyz requested changes Jun 26, 2023

View reviewed changes

heejit added 6 commits June 26, 2023 15:02

Image

2eb8804

Delete widget-date-input.png

bc286d3

Function Naming Correction

c17ed54

Widget Image

52d5200

Image link

756376e

Merge branch 'master' of https://github.com/heejit/fyne-x

22fc209

heejit added 4 commits October 5, 2023 21:24

Date input widget updated now using richtext

35fd265

Image updated

cb13369

Delete img/widget-date-input.png

e5349c4

Add files via upload

178da90

Jacalz requested changes Oct 5, 2023

View reviewed changes

heejit added 3 commits October 7, 2023 20:33

Widget name changed to JDateEntry

2966035

Add comments

Merge remote-tracking branch 'refs/remotes/origin/master'

a6356a5

TypeRune Added

f40712a

andydotxyz closed this Sep 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added date_input_widget #67

Added date_input_widget #67

heejit commented Jun 25, 2023

Jacalz commented Jun 25, 2023

heejit commented Jun 25, 2023 •

edited

Loading

heejit commented Jun 25, 2023

Jacalz commented Jun 25, 2023

heejit commented Jun 25, 2023

heejit commented Jun 25, 2023

Jacalz commented Jun 25, 2023

andydotxyz left a comment

andydotxyz Jun 26, 2023

heejit Jun 26, 2023

andydotxyz Jun 26, 2023

heejit Jun 26, 2023

andydotxyz Jun 26, 2023

heejit Jun 26, 2023

heejit Jun 26, 2023

andydotxyz commented Oct 4, 2023

heejit commented Oct 5, 2023

Jacalz left a comment

Jacalz Oct 5, 2023 •

edited

Loading

heejit Oct 7, 2023

Jacalz Oct 7, 2023

Jacalz Oct 5, 2023

Jacalz Oct 5, 2023

Jacalz Oct 5, 2023

Jacalz Oct 7, 2023

andydotxyz commented Sep 7, 2024

heejit commented Sep 8, 2024

andydotxyz commented Sep 24, 2024

heejit commented Sep 24, 2024


		const ConstDefaultDateFormat = "02-Jan-2006"

		var DayPos = []int{0, 2}


		// End: implement focusable interface

		// Start: implemnet Tappable

Added date_input_widget #67

Added date_input_widget #67

Conversation

heejit commented Jun 25, 2023

Jacalz commented Jun 25, 2023

heejit commented Jun 25, 2023 • edited Loading

heejit commented Jun 25, 2023

Jacalz commented Jun 25, 2023

heejit commented Jun 25, 2023

heejit commented Jun 25, 2023

Jacalz commented Jun 25, 2023

andydotxyz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andydotxyz commented Oct 4, 2023

heejit commented Oct 5, 2023

Jacalz left a comment

Choose a reason for hiding this comment

Jacalz Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andydotxyz commented Sep 7, 2024

heejit commented Sep 8, 2024

andydotxyz commented Sep 24, 2024

heejit commented Sep 24, 2024

heejit commented Jun 25, 2023 •

edited

Loading

Jacalz Oct 5, 2023 •

edited

Loading