WIP - Allow "Remove project" to work without JavaScript

We do this by checking if a confirm_name_input param exists (and matches the project's path). For JavaScript requests, this value is populated by filling out the modal value, and works like it always did.

For non-JS requests, this param will be empty, so we render a form back to the user to get the required confirmation.

Closes #2485 (closed)

Added 1 commit:

• ad5203ff - Remove duplicate "Project removal" spec

Reassigned to @DouweM

cc @sytses

Nice, thanks @rspeicher /cc @mikegerwitz

Great! This milestone (8.0) is set for Sep 22---is that when I can expect this change to appear on gitlab.com?

Thanks for such a quick implementation.

@mikegerwitz We do a deploy with each release candidate release, so you could see it live as soon as the 16th.

@rspeicher I don't approve this change. Separate page just for project removal with JS disabled? Now when we need to change something in project removal UI we need to check both JS-enabled and JS-disabled behaviour. This is not good. Either render separate removal page every time (with or without JS enabled) or propose better solution.

Title changed from **Allow "Remove project" to work without JavaScript ** to **WIP - Allow "Remove project" to work without JavaScript **

Title changed from **WIP - Allow "Remove project" to work without JavaScript ** to WIP - Allow "Remove project" to work without JavaScript

Making it always render a separate page regardless of JS is the easy solution, but I think that makes the UX worse for the vast majority of users (those with JS enabled).

What about using a CSS-based modal (e.g. http://drublic.github.io/css-modal/)? That way JavaScript/non-JavaScript users will see the same thing, only the submit button will be activated already from non-JavaScript users.

@stanhu I like your suggestion.

But if the submit button's already activated for those without JS, that kind of defeats the purpose of having the confirmation, no? Or did you want to keep the server-side check as well?

@rspeicher The confirmation is an extra check, in this case non-JS users will not have the benefit of this.

Making it always render a separate page regardless of JS is the easy solution, but I think that makes the UX worse for the vast majority of users (those with JS enabled).

@rspeicher I am not sure removing page would make UI worse. I personally hate modal windows and would prefer separate page. Its not common action to remove project. Also it is most clean implementation (no magical css modals and js)

@dzaporozhets If you have to remove a lot of project a page reload is less nice. Are you also open to Stan's solution detailed in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1294#note_2120583 ?

If you have to remove a lot of project a page reload is less nice

@sytses I don't think there is a difference compare to existing solution. Right now:

1. go to project settings
2. Click remove
3. type name in modal
4. Press remove button

With remove page it will be same except you will see remove form inside page content instead of modal window.

Are you also open to Stan's solution detailed in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1294#note_2120583 ?

I don't like idea of adding another UI library if we can avoid it.

We need to decide if we want removal confirmation when JS is disabled or not. If not - solution is pretty easy:

1. put real remove button on the page
2. with javascript capture click on this button with modal window
3. without JS it will remove project immediately

This is a simplest fix

@dzaporozhets thanks! that is also what @stanhu proposed in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1294#note_2120583

@stanhu if you're still up for doing this please go ahead

I agree with @dzaporozhets about this:

I don't like idea of adding another UI library if we can avoid it.

Has there been any further work on this? Thanks for the work and discussion so far.

I implemented @dzaporozhets' suggestion in !1527 (merged).

@stanhu thanks for the quick reply/change!

Thanks @stanhu, this is great!

This can be closed now that https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1527 is merged.

Status changed to closed