WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 3 years ago

Last modified 3 years ago

#22383 closed enhancement (fixed)

Combine Domain and Path UI

Reported by: scribu Owned by: jeremyfelt
Milestone: 4.3 Priority: normal
Severity: normal Version:
Component: Networks and Sites Keywords: has-patch commit dev-reviewed
Focuses: ui, multisite Cc:

Description

When editing a site in /network/site-info.php?id=123 you get two fields: one for the site's domain and another for the path.

It would make more sense from a UX perspective to have a single URL field, which would be intelligently parsed on save.

Attachments (7)

22383.diff (3.2 KB) - added by scribu 6 years ago.
Imported patch from #22235
22383.2.diff (3.6 KB) - added by ericlewis 3 years ago.
22383.3.diff (2.4 KB) - added by ocean90 3 years ago.
before.png (212.5 KB) - added by ocean90 3 years ago.
after.png (284.4 KB) - added by ocean90 3 years ago.
22383.4.diff (5.2 KB) - added by jeremyfelt 3 years ago.
22383.5.diff (2.3 KB) - added by michaelryanmcneill 3 years ago.

Download all attachments as: .zip

Change History (66)

@scribu
6 years ago

Imported patch from #22235

#1 @scribu
6 years ago

  • Keywords has-patch added

#2 @wpmuguru
6 years ago

  • Cc wpmuguru added

#3 @jeremyfelt
4 years ago

  • Component changed from Multisite to Networks and Sites
  • Focuses ui multisite added
  • Milestone changed from Future Release to 3.9

This would make me so happy. Moving to 3.9 as it definitely fits alongside domain routing and it would be great to explore options.

#4 @jeremyfelt
4 years ago

  • Milestone changed from 3.9 to Future Release

I'd like to try to make this happen in the next release. Let's try to give it some more attention early.

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


3 years ago

@ericlewis
3 years ago

#6 @ericlewis
3 years ago

attachment:22383.2.diff will apply. Also changes the Sites list table to show site URL instead of path/domain.

This ticket was mentioned in Slack in #core-multisite by eric. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-multisite by eric. View the logs.


3 years ago

#10 @ericlewis
3 years ago

Let's follow-up on the Add New Site flow in #31240

#11 @jeremyfelt
3 years ago

  • Milestone changed from Future Release to 4.2

22383.2.diff is working great. For reference, here is a screenshot of the domain/path input: https://cloudup.com/cdQ0FBdjb_r

A few observations:

  • Completely on board with showing both domain and path in the list table. Part of me wonders if having columns for each would make sense, only as a way to break things up a bit for sorting.
  • Long domain/path combos can line-break somewhat quickly. It may make sense to widen that field as the primary column (unless we split it). (Screen: https://cloudup.com/c1d5Ph3wJp1)
  • This is likely another another ticket, but adding site title to the list table would be nice. Maybe even site admin?
  • Can we include a scheme switch here as well? I'll submit WSU's "Add New Site" as an example —https://cloudup.com/chp8qHBScBd —We accept http://site/path/, https://site.path/, or site/path/ and it all just works. We could probably esc_url() before parse_url() to make sure everything is ready. The current way to scheme switch would be editing Site URL and Home under the site's settings tab.

#12 @jeremyfelt
3 years ago

Related #14172 for scheme switching.

#13 @jeremyfelt
3 years ago

It's worth looking at #17397 in the context of this ticket as well. We should ensure we're validating domains/paths in a way that makes sense across the board.

#14 @jeremyfelt
3 years ago

And another related, with #21994, which references RFC1034 regarding character count in a subdomain octet or total domain.

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


3 years ago

#16 @jeremyfelt
3 years ago

#18777 asks for a filter on the site-new.php equivalent of this. I'm not sure that we need one directly inline on site-info.php, but it's worth paying attention to.

This ticket was mentioned in Slack in #core by jeremyfelt. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


3 years ago

#19 @jeremyfelt
3 years ago

  • Milestone changed from 4.2 to Future Release

I really wanted this in 4.2, but it needs testing and some more overall strategy around it. I'll continue to push on this over the next few weeks while we ship, with the hopes of getting it in early for feedback in 4.3.

This ticket was mentioned in Slack in #core-multisite by paaljoachim. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by jeremyfelt. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


3 years ago

#24 @jeremyfelt
3 years ago

  • Milestone changed from Future Release to 4.3

Related #14172

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


3 years ago

@ocean90
3 years ago

@ocean90
3 years ago

@ocean90
3 years ago

#26 @ocean90
3 years ago

22383.3.diff is based on 22383.2.diff and includes only the change to the list table. You can see the difference in before.png and after.png which shows the current state of the global.wordpress.org network. Not sure if we really need the exta columns for domain and path, I tried to find a way to hide them per default, but couldn't find something for this.

This ticket was mentioned in Slack in #core-multisite by ocean90. View the logs.


3 years ago

#28 @jeremyfelt
3 years ago

I created #32434 to track changes to wp-admin/network/sites.php as a separate ticket.

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


3 years ago

@jeremyfelt
3 years ago

#30 follow-up: @jeremyfelt
3 years ago

Working ahead of mock-ups a bit to explore the logic.

22383.4.diff expands on the first patches:

  • If the main site, assume that nothing can be updated for scheme/domain/path on this screen.
  • If subdomain configuration, treat scheme/domain/path as one. parse_url to break them apart for update_blog_details().
  • If subdirectory configuration, assume scheme/domain are inherited from the network and only path can be updated.

I'm somewhat confused on why the option to update home/site url is necessary. It seems that multisite relies so much on the data from wp_blogs that a siteurl option that was different would cause crazy happenings.

#31 in reply to: ↑ 30 @jeremyfelt
3 years ago

Replying to jeremyfelt:

I'm somewhat confused on why the option to update home/site url is necessary. It seems that multisite relies so much on the data from wp_blogs that a siteurl option that was different would cause crazy happenings.

This dates back to mu:1168, though was included as part of another changeset. A related issue, mu:#920 also has some info. I'm still confused. :)

#32 @hugobaeta
3 years ago

@jeremyfelt asked me look into this ticket as well, and overall my thoughts align with what's proposed here. Specifically the merging of the two fields (domain and path) in the Edit Site:

https://cldup.com/r88nKEnGen.png

It seems to make the most logic, and simplifies the UI a lot. As Jeremy mentioned above, I also have no idea what that checkbox for "Update siteurl and home as well" is there for. Any way we could remove it or is there a specific use-case where is makes sense to not select that option by default??

The other side of this ticket is how we present the sites list table. The problem right now is that depending on what kind of install – sub-domain or sub-directory – we get a different UI for the table, and it gets too cluttered. My favorite solution is one stated above, where we'd only have a URL column (same for sub-domain or sub-directory), and to solve the URL breaking issue Jeremy stated above, one cool solution would be to only break the urls at "/", like so:

https://cldup.com/ZcpeUngi7Y.png

Notice how the second site breaks at the "/". We could also make that column slightly wider, since it would be the primary coluumn.

This change would make it very straight forward to understand what site it is, and would create parity when you go into “edit site” and see the URL input field match (the merged version above). No idea how this can be achieved technically, but that's the beauty of collaboration! :P Even if we can't achieve that line break at the "/", just having the parity of URL between the table view and the edit site view would be a UX win IMO. What do y'all think?

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


3 years ago

#34 @earnjam
3 years ago

Related: #32503 is about the checkbox specifically

This ticket was mentioned in Slack in #core by jeremyfelt. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by jeremyfelt. View the logs.


3 years ago

#39 @jeremyfelt
3 years ago

In 32758:

Improve code readability in site-info.php

In advance of some other work here, clean up some braces and spaces.

See #22383.

#40 @jeremyfelt
3 years ago

In 32759:

Capture domain and path when editing sites on a subdomain network

When a network is configured as subdomain, allow for the input of arbitrary domain and path combinations when editing a site rather than just the domain.

This takes a step or two toward #32503.

Props @scribu, @ericlewis, @jeremyfelt.
See #22383.

#41 @jeremyfelt
3 years ago

In 32760:

Remove the "Update siteurl and home as well" checkbox when editing a site

Rather than provide a checkbox to update the siteurl and home options, we can make an educated decision based on the current state. If the home and/or siteurl domain and path match the existing domain and path of the site, then we update with the new information.

Also, while scheme is not stored in wp_blogs along with a site, the scheme of the home and siteurl options can now be modified via the Site URL setting in site-info.php when the home and/or siteurl options match the existing domain.

Props @hugobaeta, @earnjam, @jeremyfelt.
Fixes #32503, see #22383.

#42 @jeremyfelt
3 years ago

  • Resolution set to fixed
  • Status changed from new to closed

Things are looking good. Let's close this as fixed and keep eyes open.

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by jeremyfelt. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-multisite by michaelryanmcneill. View the logs.


3 years ago

#47 @jeremyfelt
3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Per a discussion in Slack last night, we need to revisit a part of this.

Previously in multisite, a super admin could use wp-admin/network/site-info.php to change the domain and/or the path of a site whether configured as a subdomain or subdirectory install.

In 4.3, we've combined the domain and path UI for subdomain installs to make it easier to support arbitrary domain/path combinations. At the same time, we restricted subdirectory installs so that only the path could be edited. This restriction is counter to what super admins have come to expect over time.

@michaelryanmcneill is working on a patch now to to handle domain/path the same in both configurations.

#48 @jeremyfelt
3 years ago

  • Owner set to jeremyfelt
  • Status changed from reopened to assigned

#49 follow-up: @michaelryanmcneill
3 years ago

22383.5.diff implements the changes discussed last night with @jjj and @jeremyfelt as detailed above.

#50 in reply to: ↑ 49 @johnjamesjacoby
3 years ago

  • Keywords commit added

Replying to michaelryanmcneill:

22383.5.diff implements the changes discussed last night with @jjj and @jeremyfelt as detailed above.

Looks good to me.

  • Patch applies clean
  • Tests passing
  • Fixes the subdirectory install regression introduced in r32759
  • Domains & paths are parsed & saved correctly
  • Schemes, domains, & paths are correctly saved to siteurl & home

This is good to go (again) IMO.

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by jeremyfelt. View the logs.


3 years ago

#53 @ocean90
3 years ago

22383.5.diff looks good to me.

#54 @helen
3 years ago

  • Keywords dev-reviewed added

Go forth.

#55 @jeremyfelt
3 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 33586:

Multisite: Use single URL input when editing sites in a subdirectory configuration

In [32759], we adjusted site-info.php to display a single input for a site's full URL if the network was configured for subdomains. We also enforced path only editing for non-subdomain networks, which is a regression in expected behavior.

The full URL of a site can now be edited in both subdomain and subdirectory configurations.

Props @michaelryanmcneill.
Fixes #22383.

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


3 years ago

#57 @jeremyfelt
3 years ago

In 33921:

Multisite: Update help tab text for site-info.php to reference "site URL".

In [32759] and [33586], we combined the domain and path entry for a site to a single "Site URL" field. This updates the help text to reflect that.

Fixes #33748. See #22383.

This ticket was mentioned in Slack in #core-multisite by toscho. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-multisite by boone. View the logs.


3 years ago

Note: See TracTickets for help on using tickets.