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

WIP: When friendly dates are enabled, group elements use friendly dates #717

Merged

Conversation

jcarstairs-scottlogic
Copy link
Contributor

This PR is a WIP. I'm putting it up so that nobody duplicates effort. And @ransome1, feel free to let me know if this the intent of this PR is not what you want, and I'll stop working on it!

TODO:

  • Implement the feature
  • Write tests
  • Apply Prettier
  • Check all documentation is up-to-date
  • Tidy up the commit history

I love the 'friendly dates' feature. Right now, when you enable it in Settings, tags for due dates and threshold dates are displayed using localised, human-friendly words like 'today' and 'tomorrow' rather than ISO strings.

I would love it if this feature extended to 'group elements', ie the headings which appear above grouped todo items.

This PR does that. Now, when the user enables friendly dates in settings, those group elements use friendly dates. (If the user doesn't enable friendly dates in settings, nothing is different.)

Screenshot of group elements showing friendly dates for due dates

@jcarstairs-scottlogic jcarstairs-scottlogic marked this pull request as draft June 13, 2024 15:09
@ransome1
Copy link
Owner

@jcarstairs-scottlogic Hey, thank you so much for the PR. I'm currently not really available but as soon as I find some time I will look into it and report back :) Very happy to see new contributions.

@ransome1
Copy link
Owner

@jcarstairs-scottlogic I checked it out and it seems to work without any noticable issues :)

However I also agree, that the grouping of headlines, as done in the sidebar, might be necessary.

Only if grouped, it does really help the user to get a better overview of the tasks. But of course I also know, that this is not the easiest thing to achieve.

Screenshot 2024-06-14 at 11 49 33 AM

Anyhow I want to thank you for this first commit and hope we can fine grind this, as it is a nice addition.

@joeacarstairs
Copy link
Contributor

@jcarstairs-scottlogic I checked it out and it seems to work without any noticable issues :)

However I also agree, that the grouping of headlines, as done in the sidebar, might be necessary.

Only if grouped, it does really help the user to get a better overview of the tasks. But of course I also know, that this is not the easiest thing to achieve.
Screenshot 2024-06-14 at 11 49 33 AM

Anyhow I want to thank you for this first commit and hope we can fine grind this, as it is a nice addition.

I see the problem -- well spotted! I've put aside some time next week for this

@ransome1
Copy link
Owner

@jcarstairs-scottlogic let me know if I can help anyhow.

@jcarstairs-scottlogic jcarstairs-scottlogic force-pushed the group-elements-use-friendly-dates branch 2 times, most recently from e5c5fee to d4256b3 Compare July 10, 2024 11:09
In more detail, it assigns to `attributes` a new Object whose keys are
all the values of `sorting[i].value`.

These values should be all the possible attributes, so in effect, the
keys don't change.

The value assigned to each of these keys is `attributes[key]`, in other
words, the value which that key **already has** in `attributes`.

So, `attributes` should not change. So the line is redundant.

As it prevents `attributes` from being made `const`, I've removed it.
Also gently refactors updateAttributes() to take
advantage of the stronger typing
@jcarstairs-scottlogic jcarstairs-scottlogic force-pushed the group-elements-use-friendly-dates branch from d4256b3 to 7496de2 Compare July 10, 2024 11:10
@jcarstairs-scottlogic
Copy link
Contributor Author

jcarstairs-scottlogic commented Jul 10, 2024

Thanks, sorry for the inactivity. I think this should be a good first draft, though it still needs wants tests. For some reason when I run npm start I get an error because browser APIs like path and window are undefined, so I haven't been able to sanity-check this yet.

There's a bit of gentle refactoring and typing changes here, but the real meat is in the last two commits, 05b66ae and 7496de2. (The second of those is basically what I had done before.)

joeacarstairs and others added 7 commits August 4, 2024 15:17
Separates out the step which find the right
translation key for the friendly date from the
stage which translates that key.

In particular, adds a new function,
`friendlyDateTranslationKey()`, which just finds
the right translation key. `friendlyDate()` now
just calls the first function and applies the
translation function from i18next.
This will be useful for grouping by friendly date,
since we need to identify whether the attribute
key associated with a given group is a date
attribute key.
… dates are enabled.

As before, when the user hasn't got friendly dates
enabled, todo objects will be grouped by the raw
value of date attributes, hopefully an ISO string
like `2024-07-10`.

But when the user enables friendly dates, todos
will instead be grouped by the friendly date
translation key, such as `drawer.attributes.today`.
@joeacarstairs joeacarstairs force-pushed the group-elements-use-friendly-dates branch from 7496de2 to e1c5a89 Compare August 4, 2024 14:30
@ransome1
Copy link
Owner

ransome1 commented Aug 4, 2024

@jcarstairs-scottlogic thanks a bunch and sorry for being soo slow atm. I will check all commits asap.

@ransome1 ransome1 marked this pull request as ready for review August 4, 2024 16:33
@ransome1 ransome1 merged commit c3fceea into ransome1:main Aug 4, 2024
@jcarstairs-scottlogic
Copy link
Contributor Author

Woah there, thanks for looking at this, but I don't think this is ready to merge. I had realised I had made a big mistake misunderstanding how Electron works, and made a dependency across the main and renderer modules, which was causing build errors. I was in the middle of refactoring this to fix that problem yesterday. I think you might want to revert the merge for now, @ransome1?

@ransome1
Copy link
Owner

ransome1 commented Aug 5, 2024

Yeah, I am just trying to figure out, why I cannot start or build it. This might be the reason. I will revert the merge. Just let me know when you think youre good to go :)

ransome1 added a commit that referenced this pull request Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants