Skip to content

Getting started with schedules #305

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

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

Conversation

sukhwinder33445
Copy link
Contributor

@sukhwinder33445 sukhwinder33445 commented Mar 6, 2025

resolves #280

Blocked By:

#292

@cla-bot cla-bot bot added the cla/signed CLA is signed by all contributors of a PR label Mar 6, 2025
@sukhwinder33445 sukhwinder33445 force-pushed the getting-started-with-schedules branch from 1240f2f to 09eda88 Compare March 6, 2025 16:38
@sukhwinder33445 sukhwinder33445 self-assigned this Mar 6, 2025
@sukhwinder33445 sukhwinder33445 force-pushed the getting-started-with-schedules branch from 09eda88 to 63ad237 Compare March 6, 2025 16:40
Copy link
Contributor

@flourish86 flourish86 left a comment

Choose a reason for hiding this comment

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

This looks basically good already. I found some smaller issues, though.

  1. The layout of the empty state notice is broken when becoming narrow. I'd probably opt for a flex box solution.

Screen Shot 2025-03-17 at 15 34 07

  1. I find the timestamps for the scale pretty distracting, especially when the columns width becomes really narrow. We had this offline already and I see the points you made about the locale stuff. But can we probably try to decrease the line-height and fade the AM/PMs?

Something like this.

.timescale .timestamp {
   line-height: 1;
}

.timescale .timestamp > span:last-child {
    opacity: 0.5;
}
  1. Can you adjust the background of the empty-state button container? It's a little too dark. I noticed, that I used a custom value. It's #d9d9d9 in the mockup, but with 25% opacity. Probably @gray-lighter would be close, but also with 25%. I think you can use fade(@gray-light, 25%).

  2. Same for the "Add another …" Button. And the row header should be the same colors, please.
    Screenshot 2025-03-17 at 16 14 52

  3. Not sure if it's addressed in a different issue, but the row header should be the same as the body row area, when highlighted.
    Frame 36

@sukhwinder33445 sukhwinder33445 force-pushed the getting-started-with-schedules branch from 63ad237 to b920a05 Compare March 18, 2025 09:02
@sukhwinder33445 sukhwinder33445 force-pushed the add-timescale-and-indicator branch 2 times, most recently from 9f54900 to 5d70ce3 Compare March 18, 2025 09:21
@sukhwinder33445 sukhwinder33445 force-pushed the getting-started-with-schedules branch 2 times, most recently from da504df to d29b2ef Compare March 18, 2025 09:41
@flourish86
Copy link
Contributor

There's also something, that's not right here. Although there's enough space, the button and the hint element break.

Screenshot 2025-03-19 at 10 20 23

@sukhwinder33445 sukhwinder33445 force-pushed the getting-started-with-schedules branch 2 times, most recently from 44650de to 3934a9a Compare March 31, 2025 12:18
@sukhwinder33445 sukhwinder33445 force-pushed the add-timescale-and-indicator branch from 5d70ce3 to 8664a42 Compare March 31, 2025 13:01
@sukhwinder33445 sukhwinder33445 force-pushed the getting-started-with-schedules branch from 3934a9a to 8ebf977 Compare March 31, 2025 13:05
@sukhwinder33445 sukhwinder33445 force-pushed the add-timescale-and-indicator branch from 8664a42 to 2d26ce3 Compare March 31, 2025 14:10
@sukhwinder33445 sukhwinder33445 force-pushed the getting-started-with-schedules branch from 8ebf977 to 1843a0c Compare March 31, 2025 14:16
Copy link
Contributor

@flourish86 flourish86 left a comment

Choose a reason for hiding this comment

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

The "Add another rotation" version looks great.

In the empty state version, the content is not centered, though.

@sukhwinder33445 sukhwinder33445 force-pushed the add-timescale-and-indicator branch from 2d26ce3 to 90c1773 Compare April 1, 2025 08:16
@sukhwinder33445 sukhwinder33445 force-pushed the getting-started-with-schedules branch from 1843a0c to c8f3dc0 Compare April 1, 2025 08:20
@sukhwinder33445 sukhwinder33445 force-pushed the add-timescale-and-indicator branch from 90c1773 to 66111dc Compare April 1, 2025 08:31
@sukhwinder33445 sukhwinder33445 force-pushed the getting-started-with-schedules branch from c8f3dc0 to c147701 Compare April 1, 2025 08:32
@sukhwinder33445 sukhwinder33445 force-pushed the add-timescale-and-indicator branch from 66111dc to 6762542 Compare April 1, 2025 11:50
@sukhwinder33445 sukhwinder33445 force-pushed the getting-started-with-schedules branch from 8502278 to 81d245c Compare April 1, 2025 11:54
@sukhwinder33445 sukhwinder33445 force-pushed the getting-started-with-schedules branch from 81d245c to 1efb1b9 Compare April 1, 2025 12:24
flourish86
flourish86 previously approved these changes Apr 1, 2025
@sukhwinder33445 sukhwinder33445 force-pushed the add-timescale-and-indicator branch from 6762542 to 0e30729 Compare April 2, 2025 08:48
Base automatically changed from add-timescale-and-indicator to main April 2, 2025 08:56
@sukhwinder33445 sukhwinder33445 dismissed flourish86’s stale review April 2, 2025 08:56

The base branch was changed.

@sukhwinder33445 sukhwinder33445 force-pushed the getting-started-with-schedules branch 2 times, most recently from a2a5a93 to 8d5a961 Compare April 2, 2025 09:24
- Add `empty-notice` and `Add Rotation` button under `.day-header`
- Hide clock
- Add button to the .overlay element
- Overlap sidebar entry (.placeholder) with the button row
@sukhwinder33445 sukhwinder33445 force-pushed the getting-started-with-schedules branch from 8d5a961 to 5093797 Compare April 28, 2025 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla/signed CLA is signed by all contributors of a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants