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

[DX] [Form] [WebProfiler] Added abbrs with type classes #12268

Closed
wants to merge 1 commit into from

Conversation

kix
Copy link
Contributor

@kix kix commented Oct 20, 2014

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets none
License MIT
Doc PR none

This adds an <abbr/> element having a descriptive title that displays the form element's type class name. Everyone wants to know what's the type behind a form child, yes?

Looks like this:
screenshot 2014-10-21 00 22 09

@dunglas
Copy link
Member

dunglas commented Oct 20, 2014

Why <abbr>? According to https://developer.mozilla.org/en-US/docs/Web/HTML/Element/abbr it should me used for abbreviations and this is not the case here? <div> or <span> instead?

@xabbuh
Copy link
Member

xabbuh commented Oct 20, 2014

#12267 seems to be a better fix in my opinion.

@kix
Copy link
Contributor Author

kix commented Oct 20, 2014

@xabbuh, you've mixed my today's pull requests. #12267 fixes a problem giving a better solution (well, at least IMHO), while this PR adds some verbosity to the component.
@dunglas, I'm using the same pattern as used in the Events profiler tab: listener class names are wrapped with <abbr/>'s there, too.

@stof
Copy link
Member

stof commented Oct 21, 2014

@kix but in the events panel, this can be considered as an abbreviation, as the display text is the short class name, and the title is the FQCN. Here, you are mixing the name of the field and the class name of the form type being used, which are 2 different things

@stof
Copy link
Member

stof commented Oct 21, 2014

and giving the name of the form type would be better than its class name IMO anyway

@webmozart
Copy link
Contributor

I'm not sure this is needed. Hovering for 1-2 seconds is much slower than clicking the entry and checking the type on the right, no?

@webmozart webmozart added the Form label Oct 22, 2014
@kix
Copy link
Contributor Author

kix commented Oct 23, 2014

Well, technically, it's not clicking, it's the same <abbr/> there, as of Symfony 2.5.5:
screenshot 2014-10-23 11 13 54

@webmozart
Copy link
Contributor

@kix But you can see from your screenshot that the behavior is inconsistent now. Once you hover over the type name, and then over the form name. Why (from a usability perspective) should hovering over these two different things show the same tooltip?

This adds an <abbr> element having a descriptive title that
displays the form element's type class name.
@kix
Copy link
Contributor Author

kix commented Oct 27, 2014

A bit more consistent now:
screenshot 2014-10-27 15 39 24
When data.type is empty, the tip and the type name are not displayed.

@webmozart
Copy link
Contributor

Better! What does the rest of you think? @symfony/deciders

@fabpot
Copy link
Member

fabpot commented Nov 3, 2014

👍

1 similar comment
@stof
Copy link
Member

stof commented Nov 3, 2014

👍

@fabpot
Copy link
Member

fabpot commented Nov 21, 2014

Thank you @kix.

fabpot added a commit that referenced this pull request Nov 21, 2014
…(kix)

This PR was submitted for the master branch but it was merged into the 2.6 branch instead (closes #12268).

Discussion
----------

[DX] [Form] [WebProfiler] Added abbrs with type classes

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | none
| License       | MIT
| Doc PR        | none

This adds an `<abbr/>` element having a descriptive title that displays the form element's type class name. Everyone wants to know what's the type behind a form child, yes?

Looks like this:
![screenshot 2014-10-21 00 22 09](https://cloud.githubusercontent.com/assets/345754/4706604/32cf2d7a-5886-11e4-9fae-b9bff7a419ac.png)

Commits
-------

0d20753 [DX] [Form] [WebProfiler] Added abbrs with type classes
@fabpot fabpot closed this Nov 21, 2014
@kix
Copy link
Contributor Author

kix commented Nov 21, 2014

@fabpot 👍! Thanks for merging!

@kix kix deleted the profiler_type_abbrs branch November 26, 2014 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants