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

Improve error message when ID is missing #11

Closed
r-diamond opened this issue Jun 3, 2015 · 11 comments
Closed

Improve error message when ID is missing #11

r-diamond opened this issue Jun 3, 2015 · 11 comments

Comments

@r-diamond
Copy link

I created a Dynamic Sidebar, and since a) I'm used to post slugs auto-generating and b) it's been a while since I used it, I neglected to use the ID.

That led to this error screen:

image

I then had to use the back button in my browser and add the ID.

Is there any way to do an on-screen error like we do when processing payments, where the missing or incorrect box is highlighted?

(Tested this in Rainmaker, on this site: http://rebecca-rainmaker.pre-nyc2-01.rmkr.net/ Mac/Chrome)

Tagging @nathanrice but if it should go to someone else, please re-assign :D

@billerickson
Copy link
Contributor

A better option would be to make it work just as you expected it to - if no slug is provided, auto-generate it based on the name. Just pass the name through sanitize_title()

@dreamwhisper
Copy link
Contributor

A better option would be to make it work just as you expected it to - if no slug is provided, auto-generate it based on the name. Just pass the name through sanitize_title()

+1 for that

nathanrice added a commit that referenced this issue Mar 2, 2017
@nathanrice
Copy link
Contributor

f96e67d
4e5bfa7

@dreamwhisper
Copy link
Contributor

dreamwhisper commented Mar 4, 2017

Every once in awhile, I still get this message: Oops! Please choose a valid Name for this sidebar, but haven't yet sorted the steps to replicate. IDs are definitely auto generating.

When it happens, I have to refresh the screen, then I can add a sidebar with the same name I was trying.

@nickcernis
Copy link
Contributor

nickcernis commented Mar 5, 2017

I tried to reproduce this and so far only see “choose a valid Name” for '' (no value) or 0 (equates to empty).

We could try '' === $args['name'] instead of empty( $args['name'] )?

if ( '' === $args['name'] ) {
	wp_die( $this->error( 1 ) );
	exit;
}

We could also add the 'required' attribute to the name field so that it can't be submitted if it's empty.

<input required name="new_sidebar[name]" id="sidebar-name" type="text" value="" size="40" aria-required="true" />

and:

<td><input required name="edit_sidebar[name]" id="edit_sidebar[name]" type="text" value="<?php echo esc_html( $sidebar['name'] ); ?>" size="40" />

@dreamwhisper
Copy link
Contributor

@jaygidwitz could you give this a shot and just make sure it is working for you on a WP Install (develop branch - https://github.com/copyblogger/genesis-simple-sidebars/archive/develop.zip)?

I'm not getting this in my testing today so it could be my rapid fire testing Friday/Saturday. If it is good for everyone else, I'm cool with it.

@jaygidwitz
Copy link

Okay so, little problem I think:

I made a sidebar named Sidebar. (That's my creative side.)

I left the id blank. So an id of gss-sidebar-1 was generated.

Now whenever I add a widget to this sidebar it also is added to the primary sidebar with and id of sidebar

screenshot 2017-03-06 14 12 13

Noting that I was not able to replicate this with other sidebars subsequently.

Testing if this a prexisting bug in Production:

  • I cannot replicate exactly b/c when I do this with the production plugin I get the error that this issue is meant to solve.
  • When I create a new sidebar with gss 2.0.4 & name the sidebar Sidebar and give it an id of sidebar even though there is an existing sidebar with that name, then the gss sidebar replaces that one.
  • So this maybe somewhat preexisting.

I was able to replicate this again with the develop plugin but following the above instructions on a different wp install.

  • Also noting I was able to replicate @nickcernis's issue in the method he described.

@dreamwhisper I wasn't able to replicate your error other than using nick's methods.

I also don't know if my error is more of an edge case and if you would simply like a new issue for this, or if it's too much of an edge case and we don't need an issue.

@nathanrice
Copy link
Contributor

I'm curious if creating a sidebar manually with the same specs gives the same result.

Name => Sidebar
ID => gss-sidebar-1

@jaygidwitz
Copy link

Yes, testing it on a third site, it does.

screenshot 2017-03-06 15 23 56

@nathanrice
Copy link
Contributor

OK, let's get this into another issue. It might not even be fixable. Seems like WordPress basically ignores the ID and is using the Title in some instances. I'll have to investigate. But this shouldn't hold back a release, IMO.

@dreamwhisper
Copy link
Contributor

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants