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: Signature form #18477

Merged
merged 7 commits into from Oct 31, 2022
Merged

fix: Signature form #18477

merged 7 commits into from Oct 31, 2022

Conversation

DrZoidberg09
Copy link
Contributor

@DrZoidberg09 DrZoidberg09 commented Oct 19, 2022

Since version 14 the signature form is broken. This PR should fix it and bring it back to the same functionality as in version-13.

Closes: #18299
Closes: #17760
Closes: #17660

Before:

Screen.Recording.2022-10-31.at.12.25.30.PM.mov

After:

Screen.Recording.2022-10-31.at.12.08.43.PM.mov

@DrZoidberg09 DrZoidberg09 requested a review from a team as a code owner October 19, 2022 20:10
@DrZoidberg09 DrZoidberg09 requested review from shariquerik and removed request for a team October 19, 2022 20:10
@ankush ankush added the squash label Oct 20, 2022
@codecov
Copy link

codecov bot commented Oct 20, 2022

Codecov Report

Merging #18477 (a0b2824) into develop (e564dcb) will decrease coverage by 0.05%.
The diff coverage is 0.00%.

❗ Current head a0b2824 differs from pull request most recent head afd8666. Consider uploading reports for the commit afd8666 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #18477      +/-   ##
===========================================
- Coverage    63.00%   62.95%   -0.06%     
===========================================
  Files          747      749       +2     
  Lines        67794    68940    +1146     
  Branches      5996     5993       -3     
===========================================
+ Hits         42716    43403     +687     
- Misses       21618    22024     +406     
- Partials      3460     3513      +53     
Flag Coverage Δ
server-ui 31.19% <ø> (+0.02%) ⬆️
ui-tests 50.08% <0.00%> (-0.13%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@shariquerik shariquerik added the add-visuals add before and after images/ gifs to describe the fix/ feature label Oct 20, 2022
@ankush
Copy link
Member

ankush commented Oct 21, 2022

@shariquerik linked issues have visuals for reference.. seems like critical problem with signature control, lets try to fix by next release?

#18299 (comment)

Copy link
Member

@shariquerik shariquerik left a comment

Choose a reason for hiding this comment

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

I can still see 2 issues.

  1. If we create a new doc we cannot add a signature as it is blocked. After saving we can add.
  2. After saving if we reload the page then the refresh icon is missing and we can no longer edit the signature field. If we reload using reload button in the menu it makes it editable again.
  3. We should have a label for the signature field
Screen.Recording.2022-10-21.at.12.22.48.PM.mov

@DrZoidberg09
Copy link
Contributor Author

That is very strange. I use it in my timesheet doctype without any issues:

Design.ohne.Titel.mp4

Any ideas why such flaky behavior?

@shariquerik
Copy link
Member

Any ideas why such flaky behavior?

No, idea, try creating fresh doctype and check if you can reproduce

@DrZoidberg09
Copy link
Contributor Author

Yes, I could reproduce it with a new DocType. Super strange.

But I think, this should finally fix it. Could you please try?

@shariquerik shariquerik removed the add-visuals add before and after images/ gifs to describe the fix/ feature label Oct 31, 2022
@ankush ankush added the backport version-14-hotfix backport to version 14 label Oct 31, 2022
@shariquerik
Copy link
Member

shariquerik commented Oct 31, 2022

@DrZoidberg09 #17760 problem still exist

edit: Fixed

@DrZoidberg09
Copy link
Contributor Author

Awesome! Thank you @shariquerik

@shariquerik
Copy link
Member

Unrelated test failing

@shariquerik shariquerik merged commit 0bc2f16 into frappe:develop Oct 31, 2022
mergify bot pushed a commit that referenced this pull request Oct 31, 2022
Co-authored-by: Ankush Menat <ankush@frappe.io>
Co-authored-by: Shariq Ansari <sharique.rik@gmail.com>
Co-authored-by: Shariq Ansari <30859809+shariquerik@users.noreply.github.com>
(cherry picked from commit 0bc2f16)
shariquerik added a commit that referenced this pull request Oct 31, 2022
frappe-pr-bot pushed a commit that referenced this pull request Nov 1, 2022
# [14.14.0](v14.13.0...v14.14.0) (2022-11-01)

### Bug Fixes

* correct stacklevel for warnings ([#18633](#18633)) ([#18634](#18634)) ([17821f1](17821f1))
* **formatting:** use snake case for variable names ([32851ce](32851ce))
* handle exceptions thrown in post_schema_updates ([#18648](#18648)) ([#18663](#18663)) ([29a659a](29a659a))
* **insert_many:** list instead of set to maintain order ([#18641](#18641)) ([4c521b8](4c521b8))
* make module def custom checkbox readonly in prod mode ([#18698](#18698)) ([18fa732](18fa732))
* **router-js:** handle case when link is not of same host ([1b70ec4](1b70ec4))
* set proper cache key for singles when name is passed as `None` ([#18667](#18667)) ([#18668](#18668)) ([e32ba96](e32ba96))
* Signature form ([#18477](#18477)) ([8e4711b](8e4711b))
* Signature form (backport [#18477](#18477)) ([#18690](#18690)) ([34c1611](34c1611))
* support symlinked /files directory ([#18702](#18702)) ([7b8cbd0](7b8cbd0))
* use `async...await` when parsing route ([9c09c65](9c09c65))
* use `is_file_path_valid` instead of `is_safe_path` ([#18316](#18316)) ([#18642](#18642)) ([537966c](537966c))
* **UX:** Info message on workspace for better UX (backport [#18701](#18701)) ([#18703](#18703)) ([5ba85ec](5ba85ec))
* **UX:** session expiry explanation and defaults ([#18680](#18680)) ([#18685](#18685)) ([515f6c2](515f6c2))
* **UX:** warn about Etc/* timezones behaviour ([#18587](#18587)) ([#18588](#18588)) ([c8728f5](c8728f5))
* webform validation script not working ([ab2b8df](ab2b8df))

### Features

* add video conferencing option (Google Meet) to Google Calendar integration (backport [#17851](#17851)) ([#18456](#18456)) ([b04a54c](b04a54c))
* support list view for "show titles instead of name" ([#18060](#18060)) ([#18681](#18681)) ([64c2555](64c2555))

### Performance Improvements

* ensure cache works for `non_standard_user_types` when empty ([#18665](#18665)) ([#18669](#18669)) ([5f7928b](5f7928b))

### Reverts

* Revert "fix: use `is_file_path_valid` instead of `is_safe_path` (#18316) (#18642)" (#18696) ([a08c029](a08c029)), closes [#18316](#18316) [#18642](#18642) [#18696](#18696)
stephenBDT pushed a commit to alias/frappe that referenced this pull request Nov 7, 2022
Co-authored-by: Ankush Menat <ankush@frappe.io>
Co-authored-by: Shariq Ansari <sharique.rik@gmail.com>
Co-authored-by: Shariq Ansari <30859809+shariquerik@users.noreply.github.com>
(cherry picked from commit 0bc2f16)
stephenBDT pushed a commit to alias/frappe that referenced this pull request Nov 7, 2022
# [14.14.0](frappe/frappe@v14.13.0...v14.14.0) (2022-11-01)

### Bug Fixes

* correct stacklevel for warnings ([frappe#18633](frappe#18633)) ([frappe#18634](frappe#18634)) ([17821f1](frappe@17821f1))
* **formatting:** use snake case for variable names ([32851ce](frappe@32851ce))
* handle exceptions thrown in post_schema_updates ([frappe#18648](frappe#18648)) ([frappe#18663](frappe#18663)) ([29a659a](frappe@29a659a))
* **insert_many:** list instead of set to maintain order ([frappe#18641](frappe#18641)) ([4c521b8](frappe@4c521b8))
* make module def custom checkbox readonly in prod mode ([frappe#18698](frappe#18698)) ([18fa732](frappe@18fa732))
* **router-js:** handle case when link is not of same host ([1b70ec4](frappe@1b70ec4))
* set proper cache key for singles when name is passed as `None` ([frappe#18667](frappe#18667)) ([frappe#18668](frappe#18668)) ([e32ba96](frappe@e32ba96))
* Signature form ([frappe#18477](frappe#18477)) ([8e4711b](frappe@8e4711b))
* Signature form (backport [frappe#18477](frappe#18477)) ([frappe#18690](frappe#18690)) ([34c1611](frappe@34c1611))
* support symlinked /files directory ([frappe#18702](frappe#18702)) ([7b8cbd0](frappe@7b8cbd0))
* use `async...await` when parsing route ([9c09c65](frappe@9c09c65))
* use `is_file_path_valid` instead of `is_safe_path` ([frappe#18316](frappe#18316)) ([frappe#18642](frappe#18642)) ([537966c](frappe@537966c))
* **UX:** Info message on workspace for better UX (backport [frappe#18701](frappe#18701)) ([frappe#18703](frappe#18703)) ([5ba85ec](frappe@5ba85ec))
* **UX:** session expiry explanation and defaults ([frappe#18680](frappe#18680)) ([frappe#18685](frappe#18685)) ([515f6c2](frappe@515f6c2))
* **UX:** warn about Etc/* timezones behaviour ([frappe#18587](frappe#18587)) ([frappe#18588](frappe#18588)) ([c8728f5](frappe@c8728f5))
* webform validation script not working ([ab2b8df](frappe@ab2b8df))

### Features

* add video conferencing option (Google Meet) to Google Calendar integration (backport [frappe#17851](frappe#17851)) ([frappe#18456](frappe#18456)) ([b04a54c](frappe@b04a54c))
* support list view for "show titles instead of name" ([frappe#18060](frappe#18060)) ([frappe#18681](frappe#18681)) ([64c2555](frappe@64c2555))

### Performance Improvements

* ensure cache works for `non_standard_user_types` when empty ([frappe#18665](frappe#18665)) ([frappe#18669](frappe#18669)) ([5f7928b](frappe@5f7928b))

### Reverts

* Revert "fix: use `is_file_path_valid` instead of `is_safe_path` (frappe#18316) (frappe#18642)" (frappe#18696) ([a08c029](frappe@a08c029)), closes [frappe#18316](frappe#18316) [frappe#18642](frappe#18642) [frappe#18696](frappe#18696)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
3 participants