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

Double-click to select attribute text #1484

Merged
merged 2 commits into from Dec 18, 2019
Merged

Double-click to select attribute text #1484

merged 2 commits into from Dec 18, 2019

Conversation

croaky
Copy link
Contributor

@croaky croaky commented Dec 9, 2019

Each show page displays a description list of the fields.
When the field is text, it is rendered inside an inline <span>.

I've found it common to want to double-click this text
to copy-paste an identifier from our admin app into other systems.

Previously, double-clicking would not select the field's value.
The bug can be seen on the show page in the demo app. For example,
try selecting the city or zip code values from:

https://administrate-prototype.herokuapp.com/admin/orders/198962

Changing the markup to be a block <div> fixes the bug.
Alternatively, we could change the preserve-whitespace CSS rule
to be display: block;.

Each show page displays a description list of the fields.
When the field is text, it is rendered inside an inline `<span>`.

I've found it common to want to double-click this text
to copy-paste an identifier from our admin app into other systems.

Previously, double-clicking would not select the field's value.
The bug can be seen on the show page in the demo app. For example,
try selecting the city or zip code values from:

https://administrate-prototype.herokuapp.com/admin/orders/198962

Changing the markup to be a block `<div>` fixes the bug.
Alternatively, we could change the `preserve-whitespace` CSS rule
to be `display: block;`.
@croaky croaky changed the title Be able to double-click to select attribute text Double-click to select attribute text Dec 9, 2019
@pablobm
Copy link
Collaborator

pablobm commented Dec 13, 2019

@croaky - What browser are you using? I'm unable to reproduce. I have tried Firefox, Chrome and Safari on macOS, and Firefox on Linux. With and without the change, I can select the whole field with a triple click.

Incidentally, I think a better example is a Product description. The Order fields you mention are Field::String rather than Field::Text.

@croaky
Copy link
Contributor Author

croaky commented Dec 13, 2019

Chrome. Just tried triple-clicking, that works. Double-clicking doesn't.

https://administrate-prototype.herokuapp.com/admin/products/blokus

Try to select the name value "Blokus" with a double-click. The result looks like this:

Screen Shot 2019-12-13 at 3 57 26 PM

@pablobm
Copy link
Collaborator

pablobm commented Dec 14, 2019

OK, I was able to reproduce what your screenshot shows on macOS Chrome. However Product#name is not a Field::Text, but a Field::String. Therefore your PR doesn't change its behaviour. Also, I tried surrounding it with a <span> (same as text fields) and this made it work as you desire.

How about changing this to add a <span> wrapper to string fields? Please let me know if I'm missing something.

@croaky
Copy link
Contributor Author

croaky commented Dec 16, 2019

@pablobm Good call on Field::Text. It looks like we've been mostly using Field::Texts in our Administrate instance and rarely Field::Strings.

I tried using a <span> but saw the same issue as the PR description and screenshot. I pushed a follow-up change to make Field::Text act the same way as Field::String using a <div> and am seeing consistent double-click selection the way I'd expect it to behave. Kind of nice that the defaults for the two string-y types are the same?

For further background: we put Administrate in front of an existing production (follower, read-only) Postgres database, whose primary API server is written in Go. We generated some lightweight ActiveRecord models from the schema to make it all work correctly. Our particular choices for database schema or how we generated it might have resulted in some inconsistency in Field::Text vs Field::String use.

For example, some of our Field::Text fields were single-block strings like pmt123456789 or user123456789 where double-clicking would be the expectation. Doesn't visually look or feel like a paragraph to triple-click.

Our case might be a little edge so understand if you don't want to support or go with another solution. Easy enough to port this approach into our own app using the generators (already done). This has been the most reliable approach I've found so far, though, so wanted to report back upstream!

@croaky
Copy link
Contributor Author

croaky commented Dec 16, 2019

Another potential approach in admin/app/views/fields/string/_show.html.erb to avoid duplication:

<%= render 'fields/text/show', field: field %>

@pablobm
Copy link
Collaborator

pablobm commented Dec 18, 2019

On second inspection, I must have done something wrong on the first inspection, because now I can reproduce the behaviour exactly as you describe. I think the change makes sense, and that the old behaviour was a bit weird. Merging!

Thank you 👍

@pablobm pablobm merged commit c055c88 into thoughtbot:master Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants