Skip to content

UX: Allow pasting lat / lng in a single operation - #3568

Merged
lastzero merged 1 commit into
photoprism:developfrom
reubendowle:location-paste
Jul 23, 2023
Merged

UX: Allow pasting lat / lng in a single operation#3568
lastzero merged 1 commit into
photoprism:developfrom
reubendowle:location-paste

Conversation

@reubendowle

Copy link
Copy Markdown
Contributor

This speeds up the workflow when pasting location found in external mapping application

Acceptance Criteria:

  • Features and enhancements must be fully implemented so that they can be released at any time without additional work
  • Automated unit and/or acceptance tests are mandatory to ensure the changes work as expected and to reduce repetitive manual work
  • Frontend components must be responsive to work and look properly on phones, tablets, and desktop computers; you must have tested them on all major browsers and different devices
  • Documentation and translation updates should be provided if needed
  • In case you submit database-related changes, they must be tested and compatible with SQLite 3 and MariaDB 10.5.12+

This speeds up the workflow when pasting location found in external mapping application
* and lng fields
*/
let paste = (event.clipboardData || window.clipboardData).getData("text");
let s = paste.split(/[ ,]+/);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks like it might throw an exception if paste isn't a string for some reason?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The specification of this seems to indicate that you will always get an empty string result even if the data type does not support strings (https://developer.mozilla.org/en-US/docs/Web/API/DataTransfer/getData#return_value).

I tested this by pasting some non-text data (eg image data), and the code just ends up with s.length == 0, and thus ignoring it.

@lastzero

Copy link
Copy Markdown
Member

Thank you very much! I didn't know a @paste event exists and can be used with VueJS. Will this work with all browsers? Does it require further testing e.g. with browsers you haven't tested yet?

@lastzero lastzero added ux Impacts User Experience enhancement needs-work Has problems that need to be resolved labels Jul 23, 2023
@lastzero

Copy link
Copy Markdown
Member

As I'm not sure how much time I have for this next week, I'll move ahead to merge your PR and add checks in case the browser clipboard cannot be accessed for some reason. Thank you for your contribution! ❤️

@lastzero
lastzero merged commit ce13135 into photoprism:develop Jul 23, 2023
@lastzero lastzero added merged Changes are merged, but may require further testing please-test Ready for manual testing & feedback and removed needs-work Has problems that need to be resolved labels Jul 23, 2023
@lastzero lastzero changed the title Frontend: Allow paste of lat/lng in single operation UX: Allow pasting lat / lng in a single operation Jul 23, 2023
lastzero added a commit that referenced this pull request Jul 23, 2023
Signed-off-by: Michael Mayer <michael@photoprism.app>
@reubendowle

Copy link
Copy Markdown
Contributor Author

Thank you very much! I didn't know a @paste event exists and can be used with VueJS. Will this work with all browsers? Does it require further testing e.g. with browsers you haven't tested yet?

It seems you can capture any DOM event in Vue. This should work with basically any browser. They all support the paste event (https://developer.mozilla.org/en-US/docs/Web/API/Element/paste_event#browser_compatibility)

lastzero added a commit that referenced this pull request Jul 24, 2023
Signed-off-by: Michael Mayer <michael@photoprism.app>
@graciousgrey graciousgrey added tested Changes have been tested successfully and removed please-test Ready for manual testing & feedback labels Sep 8, 2023
@graciousgrey graciousgrey added the released Available in a stable release label Sep 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged Changes are merged, but may require further testing released Available in a stable release tested Changes have been tested successfully ux Impacts User Experience

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants