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

[RFC][WebProfilerBundle] Add simple placeholders into search form #10660

Merged
merged 1 commit into from Jun 6, 2014

Conversation

stloyd
Copy link
Contributor

@stloyd stloyd commented Apr 9, 2014

This is an RFC mostly, it's not a real feature, it's more like little helper for the profiler form, I guess that would mostly useful for newcomers, but sometimes even old dogs could find it useful sometimes =)

Q A
Bug fix? no
New feature? kinda
BC breaks? no
Deprecations? no
Tests pass? yes
License MIT

form

@javiereguiluz
Copy link
Member

@stloyd thanks for this pull request. I really like it! The only thing that I'm not sure about is the date placeholders. You propose to use the following:

<input type="text" name="start" id="start" value="{{ start }}"
       placeholder="i.e. {{ '-2days'|date('d.m.Y') }}">

{# ... #}

<input type="text" name="end" id="end" value="{{ end }}"
       placeholder="i.e. {{ 'now'|date('d.m.Y') }}">

In my opinion, using dynamic dates is handy but not always helpful. The problem with dates is that you never know if the first number is the day or the month. Take for example your screenshot: 07.04 is the 7th of April or the 4th of July?

I'm not sure about this but, do you think it could be better to replace the dynamic dates for date format placeholderes such as dd.mm.YYYY?

@@ -5,7 +5,7 @@
</h3>
<form action="{{ path('_profiler_search') }}" method="get">
<label for="ip">IP</label>
<input type="text" name="ip" id="ip" value="{{ ip }}">
<input type="text" name="ip" id="ip" value="{{ ip }}" placeholder="i.e. 127.0.0.1">
Copy link
Member

Choose a reason for hiding this comment

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

you mean "e.g." not "i.e."

@stloyd
Copy link
Contributor Author

stloyd commented Apr 9, 2014

@javiereguiluz Really good point, and I must say that I was not sure what to put there, so I choose most common format I know =)

But taking your point I think that this version: 9th April 2014, looks better than dd-mm-YYYY (or dd.mm.YYYY):
form
form2

@staabm
Copy link
Contributor

staabm commented Apr 9, 2014

A placeholder like 9th April 2014 could make people think they have to type the whole month-names into the field... I like the format placeholder

@romainneutron
Copy link
Contributor

e.g. 9th April 2014 is correct whereas e.g. dd-mm-YYYY is not correct (dd-mm-YYYY is a format, not an example)

As Javier proposed, I'm much more in favor of using dd-mm-yyyy as a placeholder as developers understand it, and they would write a date using the correct format.

@pborreli
Copy link
Contributor

pborreli commented Apr 9, 2014

instead of placeholders for date inputs I would use input="date" in order to benefit the features offered by browsers and html5
inputdate

@yguedidi
Copy link
Contributor

👍 for date input

@fabpot
Copy link
Member

fabpot commented Apr 15, 2014

Using a date input as suggested by @pborreli makes a lot of sense as all browsers used by developers probably support this anyway.

@fabpot
Copy link
Member

fabpot commented Jun 3, 2014

@stloyd Can you update this PR?

@stloyd
Copy link
Contributor Author

stloyd commented Jun 4, 2014

@fabpot Done, unfortunately we can't use only date type, it's not supported by IE & FF, so I have left those placeholders there with suggestion from @javiereguiluz (dd.mm.YYYY), also added usage of url, as it's support is much wider than that date type.

@fabpot
Copy link
Member

fabpot commented Jun 6, 2014

Thank you @stloyd.

@fabpot fabpot merged commit be65226 into symfony:master Jun 6, 2014
fabpot added a commit that referenced this pull request Jun 6, 2014
…search form (stloyd)

This PR was merged into the 2.6-dev branch.

Discussion
----------

[RFC][WebProfilerBundle] Add simple placeholders into search form

This is an RFC mostly, it's not a real feature, it's more like little helper for the profiler form, I guess that would mostly useful for _newcomers_, but sometimes even _old dogs_ could find it useful sometimes =)

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | kinda
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| License       | MIT

![form](https://cloud.githubusercontent.com/assets/67402/2655386/4637aa0c-bfe5-11e3-958e-84308217b52e.png)

Commits
-------

be65226 [WebProfilerBundle] Add simple placeholders into search form
@stloyd stloyd deleted the feature/webprofiler_form branch June 6, 2014 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants