Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

Conversation

@lei9444
Copy link
Contributor

@lei9444 lei9444 commented Nov 26, 2019

Description

  • For dialog errors (eg. property field ), it will navigate back to the dialog and the error property field will be highlighted with red rectangle.
  • For lu errors, it will navigate back to the dialog and luis editor will be in red rectangle
  • For lg errors, it will navigate to the lg all up view in edit mode and focus on the error line

Task Item

refs #1582

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code refactor (non-breaking change which improve code quality, clean up, add tests, etc)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Doc update (document update)

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have functionally tested my change

Screenshots

dialog

lg
lu

@lei9444 lei9444 marked this pull request as ready for review November 27, 2019 07:51
@lei9444
Copy link
Contributor Author

lei9444 commented Nov 28, 2019

Hi, @cwhitten @compulim Thanks for your review. I have resolved some of these comments. And I will add dialog's error link in this PR.

@lei9444 lei9444 requested a review from yeze322 as a code owner December 6, 2019 16:38
@lei9444
Copy link
Contributor Author

lei9444 commented Dec 6, 2019

image
Pagination is added to notification list now

@a-b-r-o-w-n a-b-r-o-w-n self-requested a review December 6, 2019 19:12
@cwhitten
Copy link
Member

cwhitten commented Dec 6, 2019

@lei9444 check out the following video: https://share.getcloudapp.com/Z4uwPKyg

I think we're still over-reporting empty fields as errors. The video shows me supplying incorrect expression syntax, but 4 errors being reported for it instead of 1. The 3 additional errors are the empty field errors that shouldn't be errors at all.

@lei9444
Copy link
Contributor Author

lei9444 commented Dec 7, 2019

@lei9444 check out the following video: https://share.getcloudapp.com/Z4uwPKyg

I think we're still over-reporting empty fields as errors. The video shows me supplying incorrect expression syntax, but 4 errors being reported for it instead of 1. The 3 additional errors are the empty field errors that shouldn't be errors at all.

Hi, @cwhitten , It's a bug in checker function, I have fixed it now. I forget to clean the message and use the error message for all properties.
image

@cwhitten cwhitten dismissed stale reviews from a-b-r-o-w-n and themself December 7, 2019 15:48

stale

@cwhitten cwhitten merged commit 844df12 into microsoft:master Dec 7, 2019
@lei9444 lei9444 mentioned this pull request Dec 9, 2019
9 tasks
@a-b-r-o-w-n a-b-r-o-w-n mentioned this pull request Dec 11, 2019
@a-b-r-o-w-n a-b-r-o-w-n mentioned this pull request Dec 11, 2019
@lei9444 lei9444 deleted the deeplink branch January 17, 2020 07:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants