appsmith icon indicating copy to clipboard operation
appsmith copied to clipboard

Published 20 hours ago • verified publisher icon appsmithorg

feat: [APPSMTH-22] Execute action on widget focus and blur

Open gitstart-appsmith opened this issue 2 years ago • 38 comments

Description

This PR enables onFocus and onBlur events on the following Widgets

  • CurrencyInput
  • Input
  • PhoneNumber
  • Select
  • MultiSelect
  • SingleTreeSelect
  • MultiTreeSelect
  • DatePicker

NB: This would require updating the Appsmith Documentation

Fixes #6452

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How Has This Been Tested?

  • Cypress Tests

Checklist:

  • [x] My code follows the style guidelines of this project
  • [x] I have performed a self-review of my own code
  • [x] I have commented my code, particularly in hard-to-understand areas
  • [ ] I have made corresponding changes to the documentation
  • [x] My changes generate no new warnings
  • [x] I have added tests that prove my fix is effective or that my feature works
  • [x] New and existing unit tests pass locally with my changes

gitstart-appsmith avatar Nov 07 '22 10:11 gitstart-appsmith

Welcome to the Appsmith community! Thank you for your first pull request and making this project better. 🤗 Please make sure that you raise a review request so your code change does not go unnoticed.

welcome[bot] avatar Nov 07 '22 10:11 welcome[bot]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
appsmith ✅ Ready (Inspect) Visit Preview Dec 9, 2022 at 5:59AM (UTC)

vercel[bot] avatar Nov 07 '22 10:11 vercel[bot]

This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected.

github-actions[bot] avatar Nov 14 '22 16:11 github-actions[bot]

@Tooluloope Could you have a look at the select type widget's changes once?

sbalaji1192 avatar Nov 23 '22 07:11 sbalaji1192

/ok-to-test sha=e8770b8

sbalaji1192 avatar Nov 24 '22 05:11 sbalaji1192

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3537871241. Workflow: Appsmith External Integration Test Workflow. Commit: e8770b8. PR: 18128. Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=18128&runId=3537871241_1

github-actions[bot] avatar Nov 24 '22 05:11 github-actions[bot]

/ok-to-test sha=d897dd3

sbalaji1192 avatar Nov 30 '22 05:11 sbalaji1192

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3580765861. Workflow: Appsmith External Integration Test Workflow. Commit: d897dd3. PR: 18128. Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=18128&runId=3580765861_1

github-actions[bot] avatar Nov 30 '22 06:11 github-actions[bot]

@gitstart-appsmith can you move the "on submit" property to be just below the 'on blur' property so that on submit and reset on submit are together in input, currency and phone widget. Apologies if this was not informed in the prd or a change from it.

Screenshot 2022-11-30 at 12 48 57 PM

dilippitchika avatar Nov 30 '22 07:11 dilippitchika

@dilippitchika QQ: what is the architecture currently if we switch page while having the input focus? https://www.loom.com/share/d0aca25f0846494b90d5d23d4f9c4e83

(pre-requisite: My input field already has the focus) I can see on switching pages, my input field triggers blur event and then again focus event to regain the old state. Is this how this should be ?

kamakshibhat-appsmith avatar Nov 30 '22 08:11 kamakshibhat-appsmith

@kamakshibhat-appsmith i think this is expected in the standard web world that once you come back refocus will trigger the event again. Let me check this once though @sbalaji1192 also need your thoughts

dilippitchika avatar Nov 30 '22 09:11 dilippitchika

@dilippitchika Also, there are some existing inconsistencies with the dropdowns. Do you think that could be a bottleneck for this feature as these inconsistencies would result in unintented onDropdownOpen/onDropdownClose events ? https://www.loom.com/share/c4955929b9f147529aefc90f9b855735

kamakshibhat-appsmith avatar Nov 30 '22 09:11 kamakshibhat-appsmith

@kamakshibhat-appsmith on page change focus trigger is also there in html world, so it's a standard behavior - https://www.loom.com/share/331063111ccc4acb86a594ae6646ab59

dilippitchika avatar Nov 30 '22 09:11 dilippitchika

@kamakshibhat-appsmith i think this is a problem with the widgets themselves and not with the onDropdownOpen/Close functionality as it also happens in production. If there's an issue already for it we can prioritise, if not please create one.

dilippitchika avatar Nov 30 '22 09:11 dilippitchika

@dilippitchika @sbalaji1192 created issue #18576 . Let's prioritise this

kamakshibhat-appsmith avatar Nov 30 '22 10:11 kamakshibhat-appsmith

@gitstart-appsmith can you move the "on submit" property to be just below the 'on blur' property so that on submit and reset on submit are together in input, currency and phone widget. Apologies if this was not informed in the prd or a change from it.

Screenshot 2022-11-30 at 12 48 57 PM

@dilippitchika Done

ghost avatar Nov 30 '22 10:11 ghost

A small hiccup w.r.t opening modal on OnDropdownOpen . https://www.loom.com/share/ebcef675e9e64f7bbb5b5728e221a2a5 @dilippitchika @sbalaji1192 Let me know how to proceed with this

kamakshibhat-appsmith avatar Dec 01 '22 07:12 kamakshibhat-appsmith

@dilippitchika This is a z-index issue. But before going there, Do you think we should provide openModal function for onFocus / onDropdownClose actions?

sbalaji1192 avatar Dec 01 '22 07:12 sbalaji1192

@sbalaji1192 we can choose not to show the action for open modal here, but is that possible today?

dilippitchika avatar Dec 02 '22 04:12 dilippitchika

@kamakshibhat-appsmith i don't think this is a blocker for this issue to proceed though. Please create a new bug for it nevertheless.

dilippitchika avatar Dec 02 '22 04:12 dilippitchika

@dilippitchika It's not possible as of today. But it should be simple to remove it from the action dropdown. but still, developer can switch to js and write the openModal function.

sbalaji1192 avatar Dec 02 '22 05:12 sbalaji1192

True unless we have a way to truly restrict some feature, this will be a limitation.

dilippitchika avatar Dec 02 '22 05:12 dilippitchika

https://github.com/appsmithorg/appsmith/issues/18620 Created issue for overlapping of dropdown with modal.

kamakshibhat-appsmith avatar Dec 02 '22 06:12 kamakshibhat-appsmith

tested and rest of the things looks good

kamakshibhat-appsmith avatar Dec 02 '22 06:12 kamakshibhat-appsmith

/ok-to-test sha=1085701

sbalaji1192 avatar Dec 05 '22 06:12 sbalaji1192

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3617908982. Workflow: Appsmith External Integration Test Workflow. Commit: 1085701. PR: 18128. Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=18128&runId=3617908982_1

github-actions[bot] avatar Dec 05 '22 07:12 github-actions[bot]

/ok-to-test sha=1085701

sbalaji1192 avatar Dec 06 '22 11:12 sbalaji1192

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3629175587. Workflow: Appsmith External Integration Test Workflow. Commit: 1085701. PR: 18128. Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=18128&runId=3629175587_1

github-actions[bot] avatar Dec 06 '22 11:12 github-actions[bot]

/ok-to-test sha=1085701

sbalaji1192 avatar Dec 07 '22 06:12 sbalaji1192

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3636363637. Workflow: Appsmith External Integration Test Workflow. Commit: 1085701. PR: 18128. Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=18128&runId=3636363637_1

github-actions[bot] avatar Dec 07 '22 06:12 github-actions[bot]