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

Add task notes first attempt #72

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Add task notes first attempt #72

wants to merge 2 commits into from

Conversation

illisan
Copy link
Contributor

@illisan illisan commented Jul 21, 2021

Summary

Describe the goal of this PR. Mention any related Issue numbers.

Requirements (place an x in each [ ])

@illisan illisan requested a review from colmdoyle July 21, 2021 00:03
@@ -24,6 +24,8 @@ module.exports = (sequelize, DataTypes) => {
},
dueDate: DataTypes.DATE,
scheduledMessageId: DataTypes.STRING,
// Unsure if this is necessary
Copy link
Contributor

Choose a reason for hiding this comment

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

You definitely need this

Blocks.Input({ label: 'Notes', blockId: 'taskNotes', optional: true }).element(
Elements.TextInput({
actionId: 'taskNotes',
maxLength: 300,
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you pick 300 at random or was their a specific reason for it?

const option = {
text: `*${task.title}*`,
value: `open-task-${task.id}`,
};
if (task.dueDate) {
option.description = `Due ${DateTime.fromJSDate(task.dueDate).toRelativeCalendar()}`;
option.description = `:spiral_calendar_pad:Due ${DateTime.fromJSDate(task.dueDate).toRelativeCalendar()}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

is :spiral_calendar_pad: part of the standard emoji set? e.g. does it render on all workspaces?

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
option.description = `:spiral_calendar_pad:Due ${DateTime.fromJSDate(task.dueDate).toRelativeCalendar()}`;
option.description = `:spiral_calendar_pad: Due ${DateTime.fromJSDate(task.dueDate).toRelativeCalendar()}`;

}
// Tried adding this line with some hardcoded text for each element but that didn't work either.
// I don't think TextInput can be passed in .elements but if that's the case documentation does not show how add mrkdwn elements to a Context block?
// Context().elements([Elements.TextInput({text:`something`})],[Elements.TextInput({text:`something else`})])
Copy link
Contributor

Choose a reason for hiding this comment

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

There's two things here, which maybe because you haven't fully written out your thoughts, but as written

  • There's nothing happening with the Context(). You're not assigning it to anything, or trying to embed it into anything. It's like a fragment of a sentence in the middle of a story.
  • I don't think you can use a Context block as an option inside an input block. You'd need to have it after the input block, which will make the list render funny.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I did some digging in the BlockBuilder source code and I think I have it.

      Context({}).elements(['Foo', 'Bar']),

Comment on lines +37 to +55
// LEAVING THIS UNTOUCHED FOR NOW

// for (start, end; start < end; start += maxOptionsLength) {
// holdingArray = openTasks.slice(start, start + maxOptionsLength);
// tasksInputsArray.push(
// Input({ label: ' ', blockId: `open-task-status-change-${start}` }).dispatchAction().element(Elements.Checkboxes({ actionId: 'blockOpenTaskCheckboxClicked' }).options(holdingArray.map((task) => {
// const option = {
// text: `*${task.title}*`,
// value: `open-task-${task.id}`,
// };
// if (task.dueDate) {
// option.description = `Due ${DateTime.fromJSDate(task.dueDate).toRelativeCalendar()}`;
// }
// return Bits.Option(option);
// }))),
// );
// }

// SANDRA TESTING
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm assuming this is just the old code, so not looking at it.

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