avoid-accidental-submissions: Keep focus and inform the user

[darkred]

Closes #4462 (based on #4462 (comment) and #4462 (comment))
The feature now:

• Prevents first enter, no focus change.
• Shows message above below the title/commit field.
• Allows a second enter on the field, currently any time later.

Test URLs

• New issue: https://github.com/sindresorhus/refined-github/issues/new
• New PR: https://github.com/sindresorhus/refined-github/compare/main...darkred:avoid-accidental-submissions-keep-focus-show-message?expand=1
• Merge PR commit: any non-draft PR
• 'create new file' commit: https://github.com/sindresorhus/refined-github/new/main

Screenshots

New issue:

New PR:

Merge PR commit:

'create new file' commit:

Notes to reviewers:

• In the last case, the message is displayed on the very top, above the filename textbox. I think it might be better if it was displayed lower, right above the commit message textbox. What do you say?
• Following fregante's comment, the message now has blue background color, instead of yellow.
• Would it be better if I used in the message <code> tags for the keys ?
  
  
• With this PR, in the new Issue/PR forms, if you press Enter while the title is empty, the message is still displayed, even though the title can't be empty. I believe it would be better if it was only displayed when there's some typed text in the textbox. Maybe this would better be as part of another PR ?
• The gif for the feature in readme.md should be updated accordingly in the end.

[fregante]

On second thought, this feels a bit the like the opposite of what it was: The prevention was silent and now it's too loud. 😅

Possible improvements:

• use blue instead of yellow (definitely)

• add a background-less smaller message immediately below the field, as previously suggested? (without the button)

  

• or with the button

  

• use a temporary overlay exactly like Chrome

  

I think the second bullet might be best because:

• it's less noisy
• it doesn't move the focus (if it was accidental, the user definitely wants to stay in the field and continue typing)

Open to more suggestions.

@darkred Either way wait until we decide before updating the readme

[yakov116]

I am leaning towards option 2.
I think this change would only be necessary for issues and PR's. I think commits and merges the old way was ok, but I am not sure.

I will try out the branch later today and let you know my final thoughts.

[fregante]

@yakov116 if this message makes sense, I think it should be shown everywhere the feature runs. It wouldn’t make sense to be visible in some places and just change the behavior silently in others.

[yakov116]

I agree. My thinking is that when merging and commiting the submit button is just below the textArea. So another button confirm right below is unnecessary.
However that is not the case with new Issues/PR's. (Its a big textArea)
I agree that we should be consistent.

I want to try out the branch to checkout the annoyance level and will give you feedback.

[yakov116]

Option 2 or 3.

• add a background-less smaller message immediately below the field, as previously suggested? (without the button)

  

• or with the button

  

But I think it has to take into account if an issue con be summited too.
Currently if I create a blank issue I get the message.

[darkred]

I have followed all your suggestions:

• it's option 2 (I used an arbitrary classname for the message element)

• Now the message in new Issues only appears when the title is not empty:
  

I tested it in all possible cases (new Issue, PRs compare, PR merge, new file, edit file) and I believe it's working ok.

[fregante]

@darkred instead of checking isIssue and empty fields, what matters to us is whether the form can be submitted. There might be other logic hidden in the form and it’s best to just verify whether the submit button is disabled. Selecting the right button on every page we cover could be troublesome but [type=submit] should bring you almost all the way there.

[darkred]

I now have added a main if, to check whether the submit button is enabled(=not disabled):

+ if (select.exists('.btn-primary[type="submit"]:not([disabled])')) {
		if (pageDetect.isNewFile() || pageDetect.isEditingFile() || pageDetect.isPRConversation()) {
			focusedInput.after(message);
		} else {
			focusedInput.parentElement!.append(message);
		}
+ }

 
Note that in PR compare pages, (e.g. https://github.com/darkred/test/compare/main...t1) for some strange reason, when having RG enabled, even with the 'avoid-accidental-submissions` feature disabled, the "Create pull request" button is initially enabled, even with empty title:

screen capture (click to expand):

But, if I disable RG, then the button is initially disabled with empty title, as expected:

screenshot (click to expand):

Update:
I used the RG's 'Find feature' procedure and I found that this is caused by the one-click-pr-or-gist feature.

[fregante]

instead of checking isIssue

[fregante]

Can you open an issue for that bug?

[darkred]

Can you open an issue for that bug?

Yes, here: #4565

[darkred]

instead of checking isIssue

I don't quite understand what you mean:
after my last changes, there's no longer check for isIssue (nor checking for empty fields) - there's now only an if check whether the page's main submit button is active.

The contained if...else :

	if (pageDetect.isNewFile() || pageDetect.isEditingFile() || pageDetect.isPRConversation()) {
	 // ..
	} else {
	 // ..

is still needed, it's for a different reason: to create the message with either .after() or parentElement.append(), as needed.

 
What more is needed for me to do?

[fregante]

Small changes required, but no gif update necessary.

[fregante]

This spacing looks great on isEditingFile:

But not so much on isNewIssue:

mt-2 mb-n1 would work better, but we need to make this change without affecting isEditingFile

---

Let's make sure the spacing is right in every page this feature runs on. Can you fix this and post some screenshots?

[darkred]

Yes, I have fixed this following your suggestion.
Here are screenshots:

New Issue

New PR/branch compare

PR merge

New file commit

Edit file commit

---

Edit: I also used .my-1 on isNewFile.

[fregante]

Looks great! Thanks for the many screenshots ☺️

[yakov116]

@fregante don't co-author by me when merging, I only added a lint.

[fregante]

Thanks @darkred!

[fregante]

I've been seen it today. It looks pleasant 😌