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

added feature to notify about recorded user interaction #60

Closed
wants to merge 14 commits into from

Conversation

aryangupta701
Copy link
Contributor

  • added a feature to let the user know weather his interaction has been recorded or not

Signed-off-by: aryangupta701 <[email protected]>
Signed-off-by: aryangupta701 <[email protected]>
Signed-off-by: aryangupta701 <[email protected]>
Signed-off-by: aryangupta701 <[email protected]>
Signed-off-by: aryangupta701 <[email protected]>
Signed-off-by: aryangupta701 <[email protected]>
@aryangupta701 aryangupta701 changed the title added feature to notify of recorded user interaction added feature to notify about recorded user interaction Aug 7, 2023
@psiinon
Copy link
Member

psiinon commented Aug 16, 2023

Now has conflicts

Signed-off-by: aryangupta701 <[email protected]>
@thc202
Copy link
Member

thc202 commented Aug 21, 2023

This needs to be updated to address conflicts.

@aryangupta701
Copy link
Contributor Author

ready for review

@thc202
Copy link
Member

thc202 commented Aug 21, 2023

Just some general feedback. I think this should be optional (it might end up being annoying). I'm also wondering if #63 is a better approach, we could do the notifications in that dialogue instead (showing a list of recorded events would be handy too). I'd have liked to be notified of ZestClientLaunch since that's one of the elements I see missing in the recordings sometimes.

@@ -17,6 +17,17 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import {
Copy link
Member

Choose a reason for hiding this comment

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

These changes could have been done in its own PR as they are independent of the notifications (I still think worth raising a new PR just adding this but fine if goes together anyway).

@aryangupta701
Copy link
Contributor Author

I think you are right we can provide that notification inside floating window itself.

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