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

Fix edit frequency #1852

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hassan254-prog
Copy link
Collaborator

Describe your changes

Display frequency whenever the user clicks on Edit
frequency_edit

Copy link
Member

@khaliqgant khaliqgant left a comment

Choose a reason for hiding this comment

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

Thanks. The "every" should show when showing an existing frequency

"hour" by itself doesn't make sense.

Also just noticed this issue, "hours" is still accepted and is a confusing unit "every hours"

freq.mov

Copy link
Contributor

@bodinsamuel bodinsamuel left a comment

Choose a reason for hiding this comment

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

Works great, minor comments

packages/webapp/src/pages/Integration/FlowPage.tsx Outdated Show resolved Hide resolved
packages/webapp/src/pages/Integration/FlowPage.tsx Outdated Show resolved Hide resolved
@hassan254-prog
Copy link
Collaborator Author

@khaliqgant , I have just updated my PR, to check for time units such as "every hours", "every days", "every mins" . It was also accepting "every mins" as an input which is a value less than 5 for minutes.

Copy link
Contributor

@bodinsamuel bodinsamuel left a comment

Choose a reason for hiding this comment

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

I think the change requested by Khaliq were unrelated to this PR and it has introduced bugs unfortunately.

  • Adding a space before every will add an other every
  • Adding a space between every and hours make the cursor jump to the end
  • Saving every hours complains about the missing unit and revert the input value

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.

None yet

3 participants