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

library: Add Calendar Entry #239

Merged
merged 5 commits into from Mar 20, 2023
Merged

library: Add Calendar Entry #239

merged 5 commits into from Mar 20, 2023

Conversation

SoNiC-HeRE
Copy link
Contributor

Added Calendar Demo under User Interface Section in Library
Issue workbenchdev/demos#3

@sonnyp sonnyp self-requested a review March 18, 2023 23:56
Copy link
Contributor

@sonnyp sonnyp left a comment

Choose a reason for hiding this comment

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

Could you also put an example of calendar marks?


Adw.StatusPage {
title: "Calendar";
description: "Keep track of current or upcoming events";
Copy link
Contributor

Choose a reason for hiding this comment

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

that'd make sense as a description for an end user calendar app but not here

I'll let you come up with a new suggestion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about : an arrangement of time into days, weeks, months, and years?

Comment on lines 16 to 27
calendar.connect("next-month", () => {
console.log("Switched to Next Month");
});
calendar.connect("next-year", () => {
console.log("Switched to Next Year");
});
calendar.connect("prev-month", () => {
console.log("Switched to Previous Month");
});
calendar.connect("prev-year", () => {
console.log("Switched to Previous Year");
});
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't really think of use case for these signals

I'd remove them - @andyholmes wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we can connect to property changes instead

calendar.connect('notify::month', () => {
  console.log(calendar.month);
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but it would be an integer , i don't think that would be relevant to log.

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right. What about we log the day/month/year in the user locale?

Comment on lines 4 to 12
let year = calendar.year;
let month = calendar.month;
let day = calendar.day;
let monthString = month.toString().padStart(2, "0");
let dayString = day.toString().padStart(2, "0");

console.log(
`Selected Date (YYYY-MM-DD): ${year}-${monthString}-${dayString}`,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

please use getDate instead - it's the perfect opportunity to showcase the DateTime API :)

https://docs.gtk.org/glib/method.DateTime.format.html

Please use this to display the date:

%x: the preferred date representation for the current locale without the time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

@sonnyp sonnyp self-assigned this Mar 19, 2023
@SoNiC-HeRE
Copy link
Contributor Author

Sure I'd do that

Copy link
Contributor

@sonnyp sonnyp left a comment

Choose a reason for hiding this comment

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

Thanks

I made some slight adjustments 40edf52

The mark UI was a bit too complicated and unclear because you had to select a day to mark the day but the day selection style would make the mark invisible for that day.

@sonnyp sonnyp merged commit e8a2c23 into workbenchdev:main Mar 20, 2023
sonnyp pushed a commit to SoNiC-HeRE/Workbench that referenced this pull request Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants