Skip to content

Remove start and end time restrictions on workshops#37439

Closed
clareconstantine wants to merge 5 commits into
stagingfrom
ws-time-limits
Closed

Remove start and end time restrictions on workshops#37439
clareconstantine wants to merge 5 commits into
stagingfrom
ws-time-limits

Conversation

@clareconstantine

@clareconstantine clareconstantine commented Oct 25, 2020

Copy link
Copy Markdown

We previously had dropdowns for the start and end times of workshop sessions that only showed times from 7:00am to 7:00pm, although you could type in any time you wanted. This range was added in this PR. The input is no longer a dropdown, so users can input their desired start and end times, and we have helpful error checking as shown in the screenshot.

Screen Shot 2020-10-27 at 11 55 54 AM

Links

Testing story

Reviewer Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@clareconstantine

Copy link
Copy Markdown
Author

@tess323 Is this the right solution? We could also set expanded limits, instead of no limits, since we probably won't have workshops starting at 4am, and you can type your time in if it's outside the range we choose. Also, should I add a note to indicate that you can select or type the times, since it's not obvious you can type in any time you want?

@molly-moen molly-moen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! I think a help tip could be useful as well.

@clareconstantine clareconstantine requested review from a team, cforkish, jamescodeorg and molly-moen and removed request for a team October 27, 2020 19:05
@clareconstantine

clareconstantine commented Oct 27, 2020

Copy link
Copy Markdown
Author

LGTM! I think a help tip could be useful as well.

@molly-moen Do you think we need a help tip if we no longer have the dropdown?

@molly-moen

molly-moen commented Oct 27, 2020

Copy link
Copy Markdown
Contributor

@clareconstantine don't think we need it with this update! Makes a lot more sense now! Out of curiosity what does a user see if they click on the clock icon?

@clareconstantine

Copy link
Copy Markdown
Author

@clareconstantine don't think we need it with this update! Makes a lot more sense now! Out of curiosity what does a user see if they click on the clock icon?

@molly-moen nothing - it's no longer clickable. I didn't remove it because it's kind of cute, but I wonder if leaving it there might confuse people who are used to clicking it to see the dropdown. I hadn't used this form in a long time, so it doesn't feel broken to me, but maybe leaving it there and not having it do anything will feel broken to users. What do you and @tess323 think?

@jamescodeorg jamescodeorg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think I vote for removing the clock button if it doesn't do anything?

@jamescodeorg

Copy link
Copy Markdown
Contributor

One more question -- do we already have tests covering the error cases that you showed in the screenshot?

@clareconstantine

Copy link
Copy Markdown
Author

One more question -- do we already have tests covering the error cases that you showed in the screenshot?

I don't think this area has much in the way of UI tests, but I will add some for TimeSelect. Thanks!

@clareconstantine clareconstantine deleted the ws-time-limits branch March 15, 2022 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants