After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 776368 - let next & previous buttons be enabled for changing month in detailed event entry dialog
let next & previous buttons be enabled for changing month in detailed event e...
Status: RESOLVED OBSOLETE
Product: gnome-calendar
Classification: Applications
Component: Edit dialog
3.22.x
Other Linux
: Normal enhancement
: 3.26
Assigned To: GNOME Calendar maintainers
GNOME Calendar maintainers
Depends on:
Blocks:
 
 
Reported: 2016-12-22 02:55 UTC by Mohammed Sadiq
Modified: 2017-11-24 22:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
This is not the final patch ,the previous and next buttons have been enabled (1.51 KB, patch)
2017-09-13 06:01 UTC, Karuna Grewal
none Details | Review
patch for making the next and previous button active for the month (5.46 KB, patch)
2017-09-13 21:19 UTC, Karuna Grewal
needs-work Details | Review
Changed patch after the review (5.89 KB, patch)
2017-09-15 05:11 UTC, Karuna Grewal
needs-work Details | Review
This patch enables switching between years, by changing months. (4.09 KB, patch)
2017-09-20 19:39 UTC, Nidhi Gupta
none Details | Review
review changes incorporated in the previous patch (5.20 KB, patch)
2017-09-21 05:59 UTC, Karuna Grewal
none Details | Review

Description Mohammed Sadiq 2016-12-22 02:55:52 UTC
In detailed event entry dialog, next button for month is disabled for December, and Previous button is enabled for January.

Ie, If the user is on December 2016, to switch to January 2017, the user have to change the year to 2017, and month to January, which requires too many user interactions. It would be nice if the user be able to click next button when the month is December that would change the month to January and increment the year by one. The same for Previous button in January too.

Thanks
Comment 1 Karuna Grewal 2017-09-13 05:02:41 UTC
(In reply to Mohammed Sadiq from comment #0)
> In detailed event entry dialog, next button for month is disabled for
> December, and Previous button is enabled for January.
> 
> Ie, If the user is on December 2016, to switch to January 2017, the user
> have to change the year to 2017, and month to January, which requires too
> many user interactions. It would be nice if the user be able to click next
> button when the month is December that would change the month to January and
> increment the year by one. The same for Previous button in January too.
> 
> Thanks

Sadiq I have done the part of being able to change the month from December to January and it's working fine. But I have slight doubt in changing the year automatically to next or previous depending on the case. I am using gcal_multi_choice_set_value to set the value for year_choice but on running it given error that assert daytime! =null fails
Can I attach my partially done patch to get some help on how can I approach the second part of year update?
Comment 2 Karuna Grewal 2017-09-13 05:08:19 UTC
(In reply to Karuna Grewal from comment #1)
> (In reply to Mohammed Sadiq from comment #0)
> > In detailed event entry dialog, next button for month is disabled for
> > December, and Previous button is enabled for January.
> > 
> > Ie, If the user is on December 2016, to switch to January 2017, the user
> > have to change the year to 2017, and month to January, which requires too
> > many user interactions. It would be nice if the user be able to click next
> > button when the month is December that would change the month to January and
> > increment the year by one. The same for Previous button in January too.
> > 
> > Thanks
> 
> Sadiq I have done the part of being able to change the month from December
> to January and it's working fine. But I have slight doubt in changing the
> year automatically to next or previous depending on the case. I am using
> gcal_multi_choice_set_value to set the value for year_choice but on running
> it given error that assert daytime! =null fails
> Can I attach my partially done patch to get some help on how can I approach
> the second part of year update? 
>*typo datetime !=null assertion fails
Comment 3 Karuna Grewal 2017-09-13 06:01:22 UTC
Created attachment 359686 [details] [review]
This is not the final patch ,the previous and next buttons have been enabled

Now we can directly toggle between january and december.
the year update is still remaining to be implemented .
Can someone please review this and guide me on how to do the second part because i'm facing error as described above.
Comment 4 Karuna Grewal 2017-09-13 21:19:26 UTC
Created attachment 359743 [details] [review]
patch for making the next and previous button active for the month

Please review this patch .Waiting for suggestions to improve it.
Comment 5 Georges Basile Stavracas Neto 2017-09-14 05:26:47 UTC
Review of attachment 359743 [details] [review]:

Thanks for your patch. There are many things to be worked before this can be merged though.

First, the whole patch does not follow GNOME Calendar Coding Style. The style is documented in HACKING.md. Please fix that.

Then, there is no commit message, only a title. And the title does not follow the GNOME Calendar commit message format. There is a template here: https://wiki.gnome.org/Newcomers/SubmitPatch.

::: src/gcal-date-chooser.c
@@ +57,3 @@
   GDestroyNotify      day_options_destroy;
 };
+GcalDateChooser *ptr_to_self;

Why is it storing a global pointer to itself?

::: src/gcal-multi-choice.c
@@ +59,3 @@
 {
+  GO_FORWARDS,
+  GO_BACKWARDS,

These should be renamed to GCAL_MULTI_CHOICE_WRAP_FORWARD/BACKWARD, and must be a separate enum.

WRAPPED must continue here.

@@ +143,3 @@
+    g_signal_emit (self, signals[GO_FORWARDS], 0);
+
+  }

Please, don't change the coding style.
Comment 6 Karuna Grewal 2017-09-15 05:11:11 UTC
Created attachment 359826 [details] [review]
Changed patch after the review

Thanks for the review.
I have tried to follow the coding style (still if there are any changes please let me know).
The global pointer's been changed.
The enum name has been changed (Please check if this is what was expected )
Comment 7 Georges Basile Stavracas Neto 2017-09-16 12:46:18 UTC
Review of attachment 359826 [details] [review]:

Some comments on the code below.

About the commit message, it's almost there. The only problems now are:
 - The title is too big, and the component name is not correct.
 - The last paragraph has capitalization typos.

::: src/gcal-date-chooser.c
@@ +432,3 @@
+  gcal_multi_choice_set_value(GCAL_MULTI_CHOICE (self->year_choice),year+1);
+  year = gcal_multi_choice_get_value (GCAL_MULTI_CHOICE (self->year_choice));
+  multi_choice_changed (self);

This is still not quite following the coding style.
- Put spaces before (
- gint, not int
- jump lines between variable declaration and assignment

::: src/gcal-multi-choice.c
@@ +59,3 @@
 {
+  GCAL_MULTI_CHOICE_WRAP_FORWARD,
+  GCAL_MULTI_CHOICE_WRAP_BACKWARDS,

The idea is to put these in a ~separate~ enum, and send one of them in the WRAPPED signal.

@@ +419,3 @@
+                  G_TYPE_NONE, 0);
+  signals[GCAL_MULTI_CHOICE_WRAP_BACKWARDS] =
+    g_signal_new ("go_back",

Don't remove the WRAPPED signal. Just add the enum as a parameter to it.
Comment 8 Nidhi Gupta 2017-09-20 19:39:26 UTC
Created attachment 360153 [details] [review]
This patch enables switching between years, by changing months.
Comment 9 Karuna Grewal 2017-09-21 05:59:55 UTC
Created attachment 360166 [details] [review]
review changes incorporated in the previous patch

I was working on this bug too.I tried the above patch to be applied but I didn't  get it working on my system. 
Here are the changes applied in my previous patch as per the previous reviews .
Waiting for the comments.
Thanks
Comment 10 Georges Basile Stavracas Neto 2017-11-24 22:06:11 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gnome-calendar/issues/97.