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

FEATURE: Don't show the 'Next Month' option in date pickers at the end of the month #15492

Conversation

AndrewPrigorshnev
Copy link
Contributor

@AndrewPrigorshnev AndrewPrigorshnev commented Jan 7, 2022

This PR affects only date-time pickers on the bookmark modal and the topic timer modal.

At the end of the month, the Monday option and the Next Month option can point to the same day or the Next Month option can even appear before the Monday option (in most cases the order is opposite):

1

This PR makes the Next Month option disappear if it is in less than 7 days from now (edit: actually Next Month works a bit better with Monday if it disappears when it is in 7 or fewer days from now, I changed it in the last commit).

Also, I did some refactoring in this PR, all steps can be seen in the commit history. The most serious step is 9f1c575. In this commit:

  • I moved hiding of dynamic options like Later Today and Later This Week to the time-shortcut-picker component. This way we won't need to write this code again and again on every form that needs it.
  • Get rid of additionalOptionsToShow setting of the time-shortcut-picker component. This setting was confusing. The component had default options to pick and these settings:
    • hiddenOptions (default options that should be hidden)
    • customOptions (additional options)
    • additionalOptionsToShow - list of default options that hidden by default but can be shown if they listed in this setting

The last one is very confusing, if the option is default it shouldn't be hidden by default. Also, it's enough to have just hiddenOptions and customOptions.

Copy link
Contributor

@eviltrout eviltrout left a comment

Choose a reason for hiding this comment

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

LGTM

@SamSaffron SamSaffron added the 2.9 label Jan 12, 2022
@AndrewPrigorshnev AndrewPrigorshnev marked this pull request as draft January 28, 2022 16:12
@AndrewPrigorshnev
Copy link
Contributor Author

After internal discussion, we decided not to hide the Next Month option. I'm closing this.

@AndrewPrigorshnev AndrewPrigorshnev deleted the feature/dont-show-the-next-month-option-at-the-end-of-the-month branch February 3, 2022 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants