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

adding properties panel #95

Merged

Conversation

JesusGuerrero
Copy link
Collaborator

@JesusGuerrero JesusGuerrero commented Sep 7, 2022

Closes #94

Add Global Editor Properties

  • General panel contains Module Name
  • Language panel contains action button to invoke language selection modal

Fixes CodeQL integration

@nysenthil
Copy link
Collaborator

Hi @JesusGuerrero I tested the code changes you made in this PR with the new backend logic. Our objective of having a Settings panel for the user to configure the module name and the language of the payload is fully met. You can plan to merge this PR.

In my testing, I made the following observations. You can decide to address them right away in this PR or at a later time based on your time availability. Thank You.

  1. In the Settings panel, I enter a module name and then directly go to the language dialog to select a language. When I return back to the Settings panel, the module name that I entered earlier is not shown there anymore. I have to enter the module name again before pressing the Save button. Current workaround is for the user to press Save before going to the language dialog.

  2. I closed the NLP Editor browser tab and then I opened a new tab to bring up the NLP Editor. If I now go to the Settings panel, it remembers the module name and the language selection I made in the already closed browser tab. Is that the expected behavior? Do you store them in cookies to remember it across different NLP Editor sessions?

  3. If the user names the module with hyphen characters in that name, it will cause problems in the backend logic. e-g: My-French-Test So, user should not have hyphen characters in the module name. However, underscores are just fine e-g: My_French_Test

@JesusGuerrero
Copy link
Collaborator Author

created the following issues Senthil
#96
#97
#98

Copy link
Collaborator

@marthacryan marthacryan left a comment

Choose a reason for hiding this comment

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

Haven't tested locally but code LGTM

Copy link
Collaborator

@laurachiticariu laurachiticariu left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Jesus!

@laurachiticariu laurachiticariu merged commit 39483d6 into CODAIT:main Sep 8, 2022
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.

New NLP Flow Global Properties Panel
4 participants