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

dev/core#5770 SK edit-in-place: focus appropriately when inline editing is initiated #32291

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

highfalutin
Copy link
Contributor

@highfalutin highfalutin commented Mar 4, 2025

Overview

Fixes dev/core#5770, at least partially - Fewer mouse clicks/keystrokes required to start editing in place in SearchKit.

Before

To actually get to a live cursor in a text field, or open the date picker, etc, multiple clicks/keystrokes are required.

After

In a table display with in-place-edit fields,

  • clicking on an editable field immediately brings focus to that particular field ("focus" means a blinking cursor, an open datepicker, etc as appropriate)
  • this works in "single field" in-place-edit mode as well as "full row" mode
  • it works with text, integer, boolean (radio, checkbox), select2 (single, whether static or entityref) and datepicker widgets.

Technical Details

My first attempt here feels slightly heavy and stiff, with hard-coded if-else blocks which probably could be refactored so that new/custom widget types could be supported, other display types could be supported, etc. But it's a step in the right direction.

Comments

  • it currently only works on table displays, afaik
  • it doesn't currently seem to work with select2 multi-select widgets
  • other widgets are untested

Copy link

civibot bot commented Mar 4, 2025

🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷

Introduction for new contributors...
  • If this is your first PR, an admin will greenlight automated testing with the command ok to test or add to whitelist.
  • A series of tests will automatically run. You can see the results at the bottom of this page (if there are any problems, it will include a link to see what went wrong).
  • A demo site will be built where anyone can try out a version of CiviCRM that includes your changes.
  • If this process needs to be repeated, an admin will issue the command test this please to rerun tests and build a new demo site.
  • Before this PR can be merged, it needs to be reviewed. Please keep in mind that reviewers are volunteers, and their response time can vary from a few hours to a few weeks depending on their availability and their knowledge of this particular part of CiviCRM.
  • A great way to speed up this process is to "trade reviews" with someone - find an open PR that you feel able to review, and leave a comment like "I'm reviewing this now, could you please review mine?" (include a link to yours). You don't have to wait for a response to get started (and you don't have to stop at one!) the more you review, the faster this process goes for everyone 😄
  • To ensure that you are credited properly in the final release notes, please add yourself to contributor-key.yml
  • For more information about contributing, see CONTRIBUTING.md.
Quick links for reviewers...

➡️ Online demo of this PR 🔗

@civibot civibot bot added the master label Mar 4, 2025
@highfalutin highfalutin changed the title SK edit-in-place: focus appropriately when inline editing is initiated dev/core#5770 SK edit-in-place: focus appropriately when inline editing is initiated Mar 4, 2025
@colemanw
Copy link
Member

colemanw commented Mar 4, 2025

Very cool @highfalutin - I like the idea of making a directive for this. There's probably some other uses for it too.

Yea it's a lot of code but it's all neatly self-contained in the directive, so this looks good to me!

@highfalutin highfalutin force-pushed the sk-in-place-tabbing branch from a136822 to 45f2f29 Compare March 4, 2025 23:35
@highfalutin
Copy link
Contributor Author

Thanks @colemanw, I just made it slightly more tidy. It could be improved on but I'd be happy to see it merged.

@highfalutin highfalutin force-pushed the sk-in-place-tabbing branch from 45f2f29 to bd745f7 Compare March 6, 2025 19:11
@highfalutin
Copy link
Contributor Author

@colemanw I made the changes you suggested and removed an unnecessary $scope variable.

@colemanw colemanw added the merge ready PR will be merged after a few days if there are no objections label Mar 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants