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 weekend styling option #182

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

Conversation

ubhaller
Copy link

@ubhaller ubhaller commented Sep 22, 2023

Add date select option to apply style to weekend days

@mikaelmello
Copy link
Owner

If I understand it right, this isn't quite an option but we're making weekends styled with red.

I feel the red conveys a quite different user experience than the existing model.

I went to check how Google Flights and Linear do it (they're like my references for the date picker lol) and Google does not highlight weekends at all, while Linear does it with the same light grey used to represent dates in the prev/next months.

I think this should be grey by default instead of red, and it should also be a configurable option (would it be on or off by default?)

@@ -695,6 +697,8 @@ pub mod date {
}
} else if date_it == today {
style_sheet = self.render_config.calendar.today_date;
} else if is_weekend(date_it) {
Copy link
Owner

Choose a reason for hiding this comment

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

It won't make a difference when both colors are the same, but in the case they're different, I think the (day in the prev/next month) style should have precedence over the (day is weekend) style.

Like in this example:
image

image

I feel the second screenshot represents a better UX when parsing the calendar

@@ -497,6 +501,7 @@ pub mod calendar {
today_date: StyleSheet::empty().with_fg(Color::LightGreen),
different_month_date: StyleSheet::empty().with_fg(Color::DarkGrey),
unavailable_date: StyleSheet::empty().with_fg(Color::DarkGrey),
weekend: StyleSheet::empty().with_fg(Color::DarkRed),
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
weekend: StyleSheet::empty().with_fg(Color::DarkRed),
weekend: StyleSheet::empty().with_fg(Color::DarkGrey),
image image

It's not that I necessarily defend grey as being the ultimate UX choice, but I think the red implies some information in a way, as if it's an unavailable date.

@@ -13,6 +13,7 @@
- Add strict clippy lints to improve code consistency and readability.
- Expand workflow clippy task to lint all-features in workspace.
- Add docs badge to readme.
- Add date select option to apply style to weekend days [#182](https://github.com/mikaelmello/inquire/pull/182).
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
- Add date select option to apply style to weekend days [#182](https://github.com/mikaelmello/inquire/pull/182).
- **Breaking**. Enabled by default, added date select option to apply style to weekend days [#182](https://github.com/mikaelmello/inquire/pull/182).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants