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

Some minor enhancements #31

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

sw-carlos-cristobal
Copy link

@sw-carlos-cristobal sw-carlos-cristobal commented Jan 18, 2023

  • Adding an option to force sorting by the due date (otherwise the items are displayed however the API returns the data unsorted)
  • Adding the due date to the lovelace card
  • Adding an option to specify the ordering by due dates (ascending/descending order)
  • Adding an option to specify the amount of days in the future to filter for the task list

image
image
image
image

This was referenced Jan 18, 2023
: item.content}
${item.content ? html`<span class="todoist-item-content">${item.content}</span>` : null}
${item.description ? html`<span class="todoist-item-description">${item.description}</span>` : null}
${item.due ? html`<span class="todoist-item-due">${item.due.date}</span>` : null}
Copy link
Owner

Choose a reason for hiding this comment

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

Won't there be a problem with margins if item's description and due are empty?

Choose a reason for hiding this comment

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

Using ? as a ternary operator is safe to use to check if those are null and to render null html if so, otherwise render the corresponding data with it. This allows the card to have all 3 of these properties as optionals.

The css could probably use a little work as I'm very amateur with front-end code so it's a little rough:

image

Copy link
Owner

@grinstantin grinstantin Jan 26, 2023

Choose a reason for hiding this comment

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

What I'm trying to say is that if description and due fields are empty, content will be rendered in a <span> with todoist-item-content class, which has top and bottom margins.

Copy link
Owner

@grinstantin grinstantin Jan 26, 2023

Choose a reason for hiding this comment

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

I don't think this is the worst option:

${item.description || item.due
    ? html`<span class="todoist-item-content">${item.content}</span>
        <span class="todoist-item-description">${[item.description, item.due?.date].filter().join(' | ')}</span>`
    : item.content}

I would be very grateful if you could test it. I'm in the process of moving and my home server with Home Assistant installed has not yet moved to a new home.

Choose a reason for hiding this comment

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

Yeah I can give it a shot sometime this week - I should have more time this weekend

Choose a reason for hiding this comment

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

Seems like it's behaving about the same as the other code I think.

Your code:
image

My previous code:
image

Copy link
Owner

@grinstantin grinstantin Jan 31, 2023

Choose a reason for hiding this comment

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

I think it's just cache. In my code, description and due.date are separated by a vertical bar.

Choose a reason for hiding this comment

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

Ah yeah I see that now, I'll try again soon(ish)

Choose a reason for hiding this comment

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

@sw-carlos-cristobal did you have some time to take a look at this?

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.

3 participants