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

feat: add date_format and hour_format settings #904

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

Conversation

JCC1998
Copy link
Contributor

@JCC1998 JCC1998 commented Apr 1, 2025

Summary

Added date_format and hour_format settings to allow customization on how to visualize dates and hours in the preview_panel
Date format selector:
image

Time format setting:
image

Tasks Completed

  • Platforms Tested:
    • Windows x86
    • Windows ARM
    • macOS x86
    • macOS ARM
    • Linux x86
    • Linux ARM
  • Tested For:
    • Basic functionality
    • PyInstaller executable

@Computerdores Computerdores moved this to 🚧 In progress in TagStudio Development Apr 1, 2025
@Computerdores Computerdores added this to the Alpha v9.5.3 milestone Apr 1, 2025
@Computerdores Computerdores added Type: Enhancement New feature or request Type: UI/UX User interface and/or user experience Type: Translations Modifies translation keys or translation capabilities. Status: Changes Requested Changes are quested to this labels Apr 1, 2025
@zfbx
Copy link
Contributor

zfbx commented Apr 2, 2025

functions perfectly well on my end. Few comments though, nothing major:

  • The dropdown can be a little confusing if you open it in the first 12 days of a month, would it be bad to add a date format along side the current date preview like MM/DD/YYYY - 04/01/2025?
  • would having optional 0 padding like in the win10 example feat: add date_format and hour_format settings #898 (review) be desirable? either by another checkbox or adding it date formats? (maybe just where it's relevant to a standardized international format as to not make the dropdown twice as long if that route was chosen?)
  • In the dropdown it seems to be sorted for the most part by the first value with exception to the first which feels out of place and is doubled later on? Is this intentional or by mistake? If intentional for functionality sake could the order be flipped to put the month-first options first to remove the dupe?
    Code_GhkLISLLwg

Just thoughts to consider, love the work though! :D I'm very much a YYYY-MM-DD person myself.

@JCC1998
Copy link
Contributor Author

JCC1998 commented Apr 2, 2025

Hi! Thank you so much for your feedback

About adding the format string on the side, I would agree, the result is:
image
If it's too much information, maybe I could use a constant date like 31-01-2025 or in american english 01-31-2025 instead of the current date

About the 0 padding, i did not find the solution on the docs: https://docs.python.org/3/library/datetime.html#strftime-and-strptime-format-codes, but searching on google I found that adding a minus sign should do the job. I can add another option as a checkbox, so the selector does not get even bigger

And about the dupped entries, it was my fault, the first one was the default option, so I started adding under it. Later I tried to do some iterations and forgot about the default, which was dupped. Thanks for the heads-up!

If you think that these changes I mentioned are correct, I will start implementing them

Thanks again!

@JCC1998
Copy link
Contributor Author

JCC1998 commented Apr 6, 2025

Hi!
I just added the zero-padding setting. Needs check in MacOs and Linux, as I don't have any machine with those OS, so i can't test if the zero padding character works. For windows i had to set the symbol as '#'
If there's something wrong or does not work properly, let me know. I also don't know how this PR work if I have to change the status so it can be reviewed again or i don't have to do anything
Thanks!

@zfbx
Copy link
Contributor

zfbx commented Apr 6, 2025

So on my mac When i change the date format and press save it crashes the program entirely leaving this as it's final log 😓

2025-04-06 05:42:14 [warning  ] [PanelModal] add_callback not implemented for SettingsPanel
python(38886,0x7ff84f26a100) malloc: Corruption of free object 0x7fa44f84f1f0: msizes 30/0 disagree
python(38886,0x7ff84f26a100) malloc: *** set a breakpoint in malloc_error_break to debug

On Win10 it seems to run fine.

Personally not sure about the naming for the zero padding. It could confuse people and it's not clear it's referring to being related to the date format. And maybe I'm nit picking but the dropdown feels difficult to read through to find a desired option, I wonder if there would be a better way to order it. Just my 2 cents however, not critical by any means.

For your reference this is what the dropdown looks like on mac for me
Screenshot 2025-04-06 at 5 49 44 AM

@JCC1998
Copy link
Contributor Author

JCC1998 commented Apr 6, 2025

About the crash, not really sure what's causing it. Did you use your mac to test the previous branch changes? Maybe the settings file has a wrong format stored. As you can see in my newest changes, i think I havent touched anything that it could break, because it was working fine for you previously, right?

About the naming of zero-padding, I can change the literal to "Date Zero Padding" or something similar, so it's more clear

About the dropdown, i need to understand why it's difficult to read: Is it too much information? It's difficult because your most used options are not the firstly shown? Would you group them by the separators ("/", ".", "-"), instead of d/m, m/d, y-m-d? Having the Year format as the whole year or the last 2 digits is confusing because it's too many options?

If you want to discuss this on discord so I can troubleshoot what's causing the crash, let me know

Thanks again for your feedback!

@zfbx
Copy link
Contributor

zfbx commented Apr 6, 2025

You're right, it seems to be me.. I might have bigger problems on my side. Ignore the Mac error, someone else will need to verify 😓

I think maybe it's abbreviated years mixed in with the full years.. and maybe the preview using the current date is busy, though I get it's purpose maybe it would be better to have a fixed date example rather than the MM/DD AND current day? maybe something with a bit of significance like first commit 04/17/2024 or The day it was announced on YouTube to be open sourced 08/21/2024? 😄 Just thoughts. I'm just a random contributor though, @CyanVoxel 's opinions might differ.. Maybe even has a better date in mind? :)

@JCC1998
Copy link
Contributor Author

JCC1998 commented Apr 6, 2025

Changed the literal of zero-padding setting and used a constant date to make it clearer by uncluttering the selector options text
Thanks for the feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Changes Requested Changes are quested to this Type: Enhancement New feature or request Type: Translations Modifies translation keys or translation capabilities. Type: UI/UX User interface and/or user experience
Projects
Status: 🚧 In progress
Development

Successfully merging this pull request may close these issues.

3 participants