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.

Sign up for GitHub

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

Jump to bottom

when creating/editing schedule, make action button available across tabs #1096

Merged
merged 13 commits into from
Nov 3, 2021
Merged

when creating/editing schedule, make action button available across tabs #1096

merged 13 commits into from
Nov 3, 2021

Conversation

ThatGalNatalie
Copy link
Contributor

@ThatGalNatalie ThatGalNatalie commented Oct 26, 2021 •
edited
Loading

PR Checklist:

  • add a short description of what's changed to the top of the CHANGELOG.md
  • add/update tests (or don't, for reasons explained below)

Describe this PR

Should resolve #1061

This moves the DictInput component and its computed prop and methods from the Schedules component into the ClockForm component

@zhen0 zhen0 added this to the UI TLC milestone Oct 26, 2021
@ThatGalNatalie ThatGalNatalie marked this pull request as ready for review October 26, 2021 19:41
zhen0
zhen0 previously requested changes Oct 28, 2021
View reviewed changes
Copy link
Member

@zhen0 zhen0 left a comment

Choose a reason for hiding this comment

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

A few small initial comments @ThatGalNatalie - let's do an in person review in our 1:1 too as i've got a few questions it may be quicker to chat through that way.

src/pages/Flow/Settings/ClockForm.vue Outdated Show resolved Hide resolved
src/components/PrefectSchedule.vue Outdated Show resolved Hide resolved
zhen0
zhen0 previously requested changes Nov 1, 2021
View reviewed changes
Copy link
Member

@zhen0 zhen0 left a comment

Choose a reason for hiding this comment

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

Thanks for the changes @ThatGalNatalie. Can you make sure master is updated? I'd also like to co-ordinate this with #1104 - can you check in with @stackoverfloweth to make sure the two don't conflict too much.

@ThatGalNatalie
Copy link
Contributor Author

ThatGalNatalie commented Nov 2, 2021 •
edited
Loading

^ Since #1104 depends on #1095 which will most likely get merged later on this week, Evan has said it's okay to merge this pr first

zhen0
zhen0 approved these changes Nov 3, 2021
View reviewed changes
Copy link
Member

@zhen0 zhen0 left a comment

Choose a reason for hiding this comment

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

Thanks @ThatGalNatalie LGTM

@ThatGalNatalie ThatGalNatalie merged commit f9b4b8f into master Nov 3, 2021
@ThatGalNatalie ThatGalNatalie deleted the issue-1061 branch November 3, 2021 19:48
@ThatGalNatalie ThatGalNatalie mentioned this pull request Nov 4, 2021
Merged
@znicholasbrown znicholasbrown mentioned this pull request Nov 9, 2021
Merged
@stackoverfloweth stackoverfloweth mentioned this pull request Dec 14, 2021
Merged
@stackoverfloweth stackoverfloweth mentioned this pull request Dec 15, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhen0 zhen0 zhen0 approved these changes

@znicholasbrown znicholasbrown Awaiting requested review from znicholasbrown znicholasbrown is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
UI TLC
Development

Successfully merging this pull request may close these issues.

when creating/editing schedule, make action button available across tabs
2 participants
@ThatGalNatalie @zhen0