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

FEATURE: add Optional countdown for e.g. deadlines #149

Merged
merged 31 commits into from
Sep 19, 2024

Conversation

merefield
Copy link
Contributor

@merefield merefield commented Sep 18, 2024

Adds an optional "deadline" option to event setup:

image

That presents a countdown on the Topic List (and Topic):

image

  • Also converts Event Form Component to gjs
  • Adds some tests

@merefield merefield changed the title Add countdown FEATURE: add Optional countdown for e.g. deadlines Sep 18, 2024
Copy link
Owner

@angusmcleod angusmcleod left a comment

Choose a reason for hiding this comment

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

@merefield Generally looks ok, however I'm still not convinced there isn't something in Discourse we could be using for the countdown display itself. See my comments.

config/settings.yml Outdated Show resolved Hide resolved
config/locales/client.en.yml Show resolved Hide resolved
@angusmcleod
Copy link
Owner

@merefield The issue was that you had added deadline as an attribute to the basic event serializer, however it was no longer an attribute on the model.

10246da

@angusmcleod
Copy link
Owner

@merefield I took the opportunity to review the UI, and I think it still needs a bit more tweaking, in addition to anything arising out of my question concerning the use of core components.

Screenshot 2024-09-18 at 19 00 56
  • It's a little cramped in the display in the composer, particularly if there's a timezone, i.e. the deadline icon is just a bit too close.
  • It there's 0 hours the hours should not be displayed at all.

@merefield
Copy link
Contributor Author

merefield commented Sep 18, 2024

@merefield The issue was that you had added deadline as an attribute to the basic event serializer, however it was no longer an attribute on the model.

10246da

@angusmcleod it's still completely broken

You cannot amend an event without a 500 error.

resolved through recreation of DB

@angusmcleod
Copy link
Owner

@merefield I'm seeing a new exception when changing the timezone when editing an event. See this video:

https://www.loom.com/share/a2b9987eccf249b59b2603cd053ebbec?sid=714ec1ef-b013-4302-88b2-5b426d048713

@merefield
Copy link
Contributor Author

@merefield I'm seeing a new exception when changing the timezone when editing an event. See this video:

https://www.loom.com/share/a2b9987eccf249b59b2603cd053ebbec?sid=714ec1ef-b013-4302-88b2-5b426d048713

thanks Angus, I'll take a look

@merefield
Copy link
Contributor Author

@angusmcleod that should be fixed now, apologies.

@angusmcleod
Copy link
Owner

@merefield Some minor style feedback

https://www.loom.com/share/1c3556960faf43d8b8d2d56ab632bbfc?sid=f81265cc-4839-4b7d-8b9f-b7a639a041e7

@merefield
Copy link
Contributor Author

merefield commented Sep 19, 2024 via email

@angusmcleod
Copy link
Owner

angusmcleod commented Sep 19, 2024

@merefield Thanks. Just doing some more checking I see that the events with deadlines in the calendar popovers need some style improvements. The addition of the deadline squishes the content too much.

Screenshot 2024-09-19 at 15 06 44

Once that's addressed I'll merge this.

@merefield
Copy link
Contributor Author

@merefield Thanks. Just doing some more checking I see that the events with deadlines in the calendar popovers need some style improvements. The addition of the deadline squishes the content too much.

Screenshot 2024-09-19 at 15 06 44 Once that's addressed I'll merge this.

I'm wondering if I simply don't show them in this view? Would that be acceptable?

@angusmcleod
Copy link
Owner

angusmcleod commented Sep 19, 2024

@merefield Maybe just push it to the next line in this view? It may be better to have consistency across all appearances of the event label.

@merefield
Copy link
Contributor Author

merefield commented Sep 19, 2024

How about this?:

image

(ignore the dev-tools highlighting)

I've pushed that change.

@merefield
Copy link
Contributor Author

merefield commented Sep 19, 2024

Improved:

image

Just for deadlines I add consistent icon right margin.

@angusmcleod
Copy link
Owner

@merefield Thanks. Approved.

@angusmcleod angusmcleod self-requested a review September 19, 2024 16:11
@angusmcleod angusmcleod merged commit 329f036 into angusmcleod:main Sep 19, 2024
5 checks passed
@merefield
Copy link
Contributor Author

thanks for the review and merge @angusmcleod !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants