Navigation Menu

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

Jump to bottom

feat/add-dynamic-feedback-form-submission #2641

Merged
merged 5 commits into from Jul 15, 2022
Merged

feat/add-dynamic-feedback-form-submission #2641

merged 5 commits into from Jul 15, 2022

Conversation

mustafapc19
Copy link
Contributor

@mustafapc19 mustafapc19 commented Jul 11, 2022 •
edited

Change Summary

Ref: #2640, #170

Change type

  • feat: (new feature for the user, not a new feature for build script)
  • fix: (bug fix for the user, not a fix to a build script)
  • docs: (changes to the documentation)
  • style: (formatting, missing semi colons, etc; no production code change)
  • refactor: (refactoring production code, eg. renaming a variable)
  • test: (adding missing tests, refactoring tests; no production code change)
  • chore: (updating grunt tasks etc; no production code change)

@mustafapc19 mustafapc19 marked this pull request as ready for review July 11, 2022 15:13
@mustafapc19 mustafapc19 added the 🛑 Status: Do Not Merge Used in PR only. The PR cannot be merged due to some reasons. label Jul 11, 2022
@mustafapc19 mustafapc19 marked this pull request as draft July 11, 2022 15:57
@mustafapc19 mustafapc19 marked this pull request as ready for review July 12, 2022 09:19
@mustafapc19 mustafapc19 removed the 🛑 Status: Do Not Merge Used in PR only. The PR cannot be merged due to some reasons. label Jul 12, 2022
wingkwong
wingkwong requested changes Jul 12, 2022
View reviewed changes
packages/nc-gui/components/FeedbackForm.vue Outdated Show resolved Hide resolved
packages/nocodb/src/lib/meta/api/utilApis.ts Outdated Show resolved Hide resolved
wingkwong
wingkwong reviewed Jul 13, 2022
View reviewed changes
Copy link
Member

@wingkwong wingkwong left a comment

Choose a reason for hiding this comment

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

The logic looks fine but the naming for isShown is confusing. If users don't close it, isShown is false while the form actually is shown.

@wingkwong wingkwong requested a review from mertmit July 13, 2022 09:10
@mustafapc19 mustafapc19 marked this pull request as draft July 14, 2022 12:34
@mustafapc19 mustafapc19 added the 🛑 Status: Do Not Merge Used in PR only. The PR cannot be merged due to some reasons. label Jul 14, 2022
@mustafapc19 mustafapc19 marked this pull request as ready for review July 15, 2022 09:44
@wingkwong wingkwong requested a review from mertmit July 15, 2022 11:00
@o1lab o1lab merged commit 4921c4e into develop Jul 15, 2022
@wingkwong wingkwong deleted the feat/2640-add-dynamic-feedback-form-submission branch July 15, 2022 14:15
@wingkwong wingkwong removed the 🛑 Status: Do Not Merge Used in PR only. The PR cannot be merged due to some reasons. label Jul 15, 2022
@wingkwong wingkwong added this to the Next Release : v0.92.x milestone Jul 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@o1lab o1lab o1lab left review comments

@wingkwong wingkwong wingkwong approved these changes

@mertmit mertmit Awaiting requested review from mertmit

Assignees

@wingkwong wingkwong

Labels
None yet
Projects
None yet
Milestone
Feature Release : 0.92.4
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@mustafapc19 @o1lab @wingkwong @mertmit