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

Create a task from a message #8375

Merged
merged 1 commit into from
May 4, 2023
Merged

Create a task from a message #8375

merged 1 commit into from
May 4, 2023

Conversation

hamza221
Copy link
Contributor

@hamza221 hamza221 commented Apr 13, 2023

closes #6127

image

@hamza221
Copy link
Contributor Author

@ChristophWurst lmk if you think task.js and calendar.js should be moved calendar-js when you find time to review it

@ChristophWurst
Copy link
Member

@hamza221 if you can't pinpoint the warning and it doesn't influence the usability of the new feature create a ticket and we'll take a deeper dive in the stabilization phase. Let's get this feature in :shipit:

@hamza221 hamza221 marked this pull request as ready for review May 2, 2023 17:49
@ChristophWurst
Copy link
Member

conflicts

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Code looks good. Didn't test

@ChristophWurst
Copy link
Member

eslint needs a fix

src/task.js Outdated Show resolved Hide resolved
@JohannesGGE
Copy link
Contributor

Screenshot from 2023-05-03 12-07-47_
I was a bit confused at first because I used the action-menu on the right and couldn't find the option. Maybe it should be added there too?

@JohannesGGE
Copy link
Contributor

image
I noticed some things:

  • The headline is "Create event", is that intended?
  • I cannot select the calendar to which the task should be added (list is empty)
  • Unlike the create event, it is not clear that the datetimes are "Start date" and "Due date" (at least for me) (most of the time I personally set only a due date)
  • "Tell us your story:" is the description field? If yes, I would just write "Description" to prevent confusion.

@ChristophWurst
Copy link
Member

Create should be disabled if you have no calendars with VTODO support, I guess

@hamza221
Copy link
Contributor Author

hamza221 commented May 3, 2023

Create should be disabled if you have no calendars with VTODO support, I guess

on it

@ChristophWurst
Copy link
Member

@JohannesGGE please re-review :)

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

  • The height of the description field could be increased so that it’s nicer to type in there, and that the "Create" button doesn’t float in the air but is exactly at the bottom.

Copy link
Contributor

@JohannesGGE JohannesGGE left a comment

Choose a reason for hiding this comment

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

I would format the date+time (all day disabled) like the "create event" does it. Right now there is just the time shown.
Screenshot from 2023-05-04 12-02-17
image

</Multiselect>

<br>
<button class="primary" :disabled="saving" @click="onSave">
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<button class="primary" :disabled="saving" @click="onSave">
<button class="primary" :disabled="disabled" @click="onSave">

@ChristophWurst
Copy link
Member

Nit: Select option -> Select calendar
Nit: List is empty -> No calendars with task list support

@hamza221
Copy link
Contributor Author

hamza221 commented May 4, 2023

image

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Works

@ChristophWurst
Copy link
Member

Please squash

Signed-off-by: hamza221 <hamzamahjoubi221@gmail.com>
@ChristophWurst ChristophWurst merged commit 7b48af2 into main May 4, 2023
@ChristophWurst ChristophWurst deleted the feature/mail2task branch May 4, 2023 22:44
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.

Convert Email to Task
4 participants