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

Settings: design for Date Format and Time Format site settings #10441

Closed
retrofox opened this issue Jan 5, 2017 · 29 comments
Closed

Settings: design for Date Format and Time Format site settings #10441

retrofox opened this issue Jan 5, 2017 · 29 comments
Assignees
Labels
[Feature] Site Settings All other general site settings. [Type] Task

Comments

@retrofox
Copy link
Contributor

retrofox commented Jan 5, 2017

We can set these parameters using /wp-admin into the settings page - general options (/wp-admin/options-general.php)
image

So the idea is to be able to set these values from Calypso. I guess that it should be placed next to timezone settings:

image

This is the link to get this section: http://calypso.localhost:3000/settings/general/$site

So the first thing that we need is a nice design :-D

cc @iamtakashi

@retrofox retrofox added the [Feature] Site Settings All other general site settings. label Jan 5, 2017
@lancewillett lancewillett changed the title Site: Design Date Format and Time Format Settings: design for Date Format and Time Format site settings Jan 6, 2017
@iamtakashi
Copy link
Contributor

iamtakashi commented Jan 11, 2017

Here is an idea for the setting.

1

It uses straightforward radios, but note that there are no inline input fields like wp-admin for custom format options – It’s hidden by default to avoid visual clutter for the majority of our users who likely to pick other options. The examples in the options are real time based on the site timezone like wp-admin.

When you choose “Custom”, each displays an input form with SelectDropdowns that add Tokens when you select a format. Much like SEO settings for Business plan users.

2

You can create a custom format by selecting a format you want to use in the order you want to display. Once a token is in the field, you can see the preview below.

3

SelectDropdowns are limited to common ones, but since it’s an input field, you can also input directly characters such as commas and colons, even the less common ones such as “T” (Timezone abbreviation) to achieve parity with wp-admin. (Below is just an example, and you don't need to add all 4 of them.)

4

@iamtakashi iamtakashi added the [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR label Jan 11, 2017
@retrofox
Copy link
Contributor Author

Very nice, great job Takashi. I like it.
Some, maybe silly, questions:

  1. I see that we have four options (dropdowns) for both formats (date and time). What could happen if the user wanna use only one custom value, let's say Monday. I guess that he/she could delete/edit them directly from the input field.
    image

  2. What about if he/she wants to add more than four custom values. Make sense wondering this?

  3. I see that we aren't using native select elements. I really like these customs dropdowns, but we've had to re-implement some components using the native ones thinking on mobile devices. We could switch custom-dropdown by native-select-element but the design should support this behaviour.

@Copons
Copy link
Contributor

Copons commented Jan 11, 2017

Uh! Nice ones @iamtakashi!

A week ago or so I've started working on the code-side of this: #10400
Nothing fancy, just a port of the wp-admin side, but if you want to work on this, there could be something you may want to reuse.

@iamtakashi
Copy link
Contributor

iamtakashi commented Jan 11, 2017

@retrofox,

  1. Note the input field is blank to start with. So, if they only want to have one custom value, they only need to select the value to add into the input. In your example, they would only need to select one of Day of Week. They can also delete/edit directly if they change mind.

  2. They can have as many/few as they wish. I hope my answer to 1. clarifies this too.

  3. I intended to use the native SelectDropdown but the compact one. It's true that I reduced some padding and added a plus sign at the beggining of header text. Would that cause issues? I might be misunderstanding your point here.

@retrofox
Copy link
Contributor Author

nice @Copons.
We can join efforts here. I could continue your branch. I've seen the changes and it looks good to me. Otherwise, you could apply these changes and continue with this job?
Feel free to take whatever you want. thanks! :-D

@iamtakashi
Copy link
Contributor

iamtakashi commented Jan 11, 2017

@Copons,
Oh, I had no idea. Thanks for letting us know!

@iamtakashi
Copy link
Contributor

@retrofox,
Ah, you probably meant "browser native". Would our "Calypso native" (that's what I meant :)) SelectDropdown would cause issues on mobile?

@retrofox
Copy link
Contributor Author

retrofox commented Jan 11, 2017

I intended to use the native SelectDropdown but the compact one. It's true that I reduced some padding and added a plus sign at the beggining of header text. Would that cause issue? I might be misunderstanding your point here.

I guess so. I mean to native elements to those elements that are natively supported by the device. The browser in this case.
For instance, when we did the timezone selector the first approach was done using the select-dropdown (which isn't a native element since it's done using another html elements plus js, etc). Now the timezone looks like this:

So what I suggest is, if it worth, switch the selectors depending on the width of the viewport or the device ... or simply implement the selectors using the native ones. Make sense?

@retrofox
Copy link
Contributor Author

SelectDropdown would cause issues on mobile?

Yes ...

@Copons
Copy link
Contributor

Copons commented Jan 11, 2017

@retrofox @iamtakashi No worries! I pretty much picked this from the wp-admin parity list without telling anyone. 😄

I guess I could continue working on it, since the "tokenized" approach looks quite like the one used for the SEO title structure, and I know who to bug if I need help on them.

Either way, the main struggle is converting between PHP and Moment.js formats.
Moment uses very different tokens and doesn't even support some of the PHP ones.

@iamtakashi
Copy link
Contributor

@retrofox Thanks for clarifying. I've looked #5231. Although the number of options are much less than language or timezone settings discussed there, it makes sense to go for the device native one all viewport size for this setting too. It looks like we can style it at least its face, so I'm ok with it.

@folletto
Copy link
Contributor

Great discussion here. My only comment at this stage would be to avoid having all of this inside the same "General" settings page, but having a button / link / LinkableCard. In that way the page is shorter and clearer, and this whole UI gets the clarity of being alone in a sub-page. :)

@Copons
Copy link
Contributor

Copons commented Jan 13, 2017

@folletto That makes sense.
I've seen in Settings / Writing there's an already established pattern going on (the two taxonomy linkable cards right at the beginning).

I'd say I could group all date and time settings in a linkable card right below "Language".
Something like:
screen shot 2017-01-13 at 16 23 30

That opens a sub-page like this:
screen shot 2017-01-13 at 16 26 11


In fact, it could even be a full fledged "Locale" sub-page that includes the Language option too, but I'd argue that Language is a much more important setting that should stay in the most prominent position possible.

@folletto
Copy link
Contributor

I'd say I could group all date and time settings in a linkable card right below "Language".

I'd go with that option, yes. 👍

Tiny detail: would be perfect if the subtitle of the linked card contained the current format, like:

screen shot 2017-01-13 at 21 29 41

@retrofox
Copy link
Contributor Author

Side question: do we have a way to scroll the page to a specific card? In other words, can we position a specific section of this page when the user comes from a link? Similar way when the link has # in its url.

@Copons
Copy link
Contributor

Copons commented Jan 13, 2017

@retrofox I'm not exactly sure, since I guess React components aren't loaded yet when the browser tries to jump to an # anchor. We could maybe scroll to an element after its mount?

If instead you're talking specifically about settings sub-pages, such as the taxonomies ones and the proposed date/time one, there is already a system in place doing that (see: https://wordpress.com/settings/taxonomies/category/{site}), that I guess I could easily bring over for the date/time.

@Copons Copons self-assigned this Jan 16, 2017
@Copons
Copy link
Contributor

Copons commented Jan 16, 2017

After some days of working on this I've noticed a couple of things that I believe should be tackled in a follow up PR to avoid over-bloating the initial one (that will then stick to a carbon-copy parity between WP Admin and Calypso):

  • Tokenized custom fields
    There are two options already available in Calypso: the TokenField used for tags and the TitleFormatEditor used for SEO title structures.
    The first does not allow for free text, and the second is a Draft.js implementation deeply tied to the title structure. Ideally I would need to customize TitleFormatEditor to the point of making it useable for this purpose. It's not easy though, and we have the additional complexity of having to convert twice the formats: Draft.js tokens -> Moment -> PHP.

  • Date and time formats own sub-page
    Obviously enough, taxonomies are not site settings, therefore their sub-pages are stripped down of the entire logic we have in place for handling the site settings.
    Moving the date/time formats into a new page means duplicating a considerable amount of stuff over from site-settings/form-general.jsx.

In both case, a short-ish and manageable-ish PR would instead become daunting.
I'd say: keep working on the current changes in #10400 (which also entail some backend changes), hide them behind a feature flag and later iterate and improve.

EDIT: rebasing, I've noticed that form-general.jsx has been wrapped in an HOC. I may be able to reuse that HOC for the subpage too.

@retrofox
Copy link
Contributor Author

@Copons I'm sorry for the delay to answer.
I do not completely understand what are you trying to say. Should we forget/get rid of some piece of the design here?

@Copons
Copy link
Contributor

Copons commented Jan 18, 2017

@retrofox Oh no, not at all!
Just, I'd rather tackle tokens and sub-pages in separate PRs as they're likely gonna be quite beefy.

@iamtakashi
Copy link
Contributor

iamtakashi commented Jan 18, 2017

I'm also confused what's been proposed here.

Anyway, I strongly feel that we should avoid a carbon copy of the setting in wp-admin since the custom format setting feels like only for programmers, and difficult to use for the majority of our users.

I'm not saying my idea is perfect, and there might be a better idea, but we should avoid a carbon copy parity for the setting.

@iamtakashi
Copy link
Contributor

iamtakashi commented Jan 18, 2017

As soon as I post the previous comment I saw the clarification :/ It looks like we've misinterpreted the suggestion. Thanks for clarifying @Copons.

@Copons
Copy link
Contributor

Copons commented Jan 18, 2017

I believe I'm explaining poorly.
I'm only proposing of splitting the development of this feature in several steps to avoid over-bloating single PRs.

Step 1: carbon copy (pretty much what I have already done anyway)
Step 2: move the date/time formats in their own sub-pages
Step 3: add token fields
Step 4: ship to prod


Ah! Damn simultaneous comments! 😄

@melindahelt
Copy link

Adding a suggestion that I user submitted at 3025440-t to display UTC + City together in dropdown.

@mtias
Copy link
Member

mtias commented Mar 8, 2017

I stumbled on this while checking my site settings and the amount of vertical space we are taking for seldomly used settings doesn't seem great.

I don't think it warrants its own page, but what about combining all these under a segmented control navigation?

image

cc @iamtakashi @folletto @Copons @retrofox

@Copons
Copy link
Contributor

Copons commented Mar 8, 2017

@mtias Date/time/week sub-page is pretty much ready to go (and I'd say it could also include the Timezone setting in a future PR) 🙂
#11259

Also, it's almost essential to introduce the date/time format "tokenization", since I need to add a lot of new stuff and doing so in form-general.jsx would be utter madness.

@mtias
Copy link
Member

mtias commented Mar 8, 2017

I see, so this was being worked on separately twice?

@Copons
Copy link
Contributor

Copons commented Mar 8, 2017

@mtias What do you mean?
The decision to move in a sub-page came up when I was almost ready to ship the date-time-week in form-general so I chose to merge that anyway and then move it in the sub-page (without touching the Timezone part that was (is?) worked on by other people).

This said, nothing stops us to add the segmented control to the sub-page.
Just, please please please please, I need the sub-page as a sort of clean slate to work on the tokenization!

@lancewillett
Copy link
Contributor

Is this related to work in #11316?

@Copons
Copy link
Contributor

Copons commented Jul 25, 2017

@lancewillett Nope, while this internally uses the locale settings for preview purposes, it only takes care of the date/time formats used in themes.

@matticbot matticbot removed the [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR label Mar 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Site Settings All other general site settings. [Type] Task
Projects
None yet
Development

No branches or pull requests

9 participants