one-click-pr-or-gist: In PR compare pages the "Create pull request" button is always enabled, even with empty title

[darkred]

Please ensure:

• The bug is caused by Refined GitHub. It doesn't happen if I disable the extension.

Describe the problem and how to replicate it

Continuing #4556 (comment)

In PR branch compare pages, e.g. https://github.com/darkred/test/compare/main...t2?expand=1, the "Create pull request" button is always enabled, even with empty title - (the expected behavior is to be disabled when the title is empty).
Disabling the one-click-pr-or-gist feature fixes the problem.

See:

• the attached screen capture for the issue, and
• the screenshot for how it is (as expected) with the one-click-pr-or-gist feature disabled.

Screenshots/video/gif showing the issue, if it’s visual

Console errors, if any

No response

Example URL

https://github.com/darkred/test/compare/main...t2?expand=1

Browser(s) used

Chrome 91, Firefox 90

[fregante]

If we can fix this, it has to somehow hook into GitHub’s code so that GitHub changes the state of these buttons autonomously

[darkred]

Note regarding Gist pages:

The one-click-pr-or-gist feature also applies to isGist pages (https://gist.github.com/).
In such pages, you can't create a new Gist having the 'filename' field empty, otherwise you get a 'Contents can't be empty' message.
But, even with Refined GitHub disabled, the 'Create secret Gist' button is always enabled, even with 'filename' empty. See screenshots:

one-click-pr-or-gist disabled/enabled (button is always enabled):

one-click-pr-or-gist disabled, try to submit with 'filename' empty:

In other words, the fact that the button is always enabled in Gists, could be considered a GitHub 'annoyance'.
So, given this issue, I'd suggest that the 'Create secret Gist' separate button to also behave as expected:
i.e. to be disabled when the 'filename' field is empty.

[fregante]

i.e. to be disabled when the 'filename' field is empty.

I'd treat that as an enhancement more than a bug fix since the behavior is not there to begin with, whereas PRs do show a disabled button by default.

[darkred]

If we can fix this, it has to somehow hook into GitHub’s code so that GitHub changes the state of these buttons autonomously

 
I tried investigating this, and this is what I came up with:

• in PR branch compare pages, e.g. https://github.com/darkred/test/compare/main...t2?expand=1
  the separate "Create pull request" button doesn't have missing/less classes than that of the initial/combined one.
  In other words, making it get the exact same classes, doesn't fix the problem. Here are the class lists for comparison:
  btn btn-primary BtnGroup-item hx_create-pr-button js-sync-select-menu-button flex-auto (initial/combined)
  btn btn-primary ml-2 tooltipped tooltipped-s (separate)

• I used Chrome's devtools DOM change breakpoint ('Attributes modifications') on the initial combined 'Create pull request' submit button, and then typed something on (or emptied) the PR title input, and as the breakpoint was triggered I got this stack trace:

  It (html-validation.ts:100)
  n (html-validation.ts:75)
  

  which refers to these two document's functions (the arrows highlight the current line) :

  // Validate a form or input.
  export function validate(form: HTMLInputElement | HTMLFormElement) {
    const validity = form.checkValidity()
    for (const button of form.querySelectorAll<HTMLButtonElement>('button[data-disable-invalid]')) {
      button.disabled = !validity    // <----
    }
  }

  and

  // Observe required fields and validate form when their validation state
  // changes.
  onFocus(supportedFields, field => {
    let previousValid = (field as Field).checkValidity()
    function inputHandler() {
      const currentValid = (field as Field).checkValidity()
      if (currentValid !== previousValid && (field as Field).form) {
        validate((field as Field).form!)    // <----
      }
      previousValid = currentValid
    }
  
    field.addEventListener('input', inputHandler)
    field.addEventListener('blur', function blurHandler() {
      field.removeEventListener('input', inputHandler)
      field.removeEventListener('blur', blurHandler)
    })
  })

    
  So, since the RG extension cannot access the document's functions, I think that the way to fix this is, is to make our own custom function that would activate on focus on the title input element, and if the title is empty, to remove the disabled attribute from the separate submit button (screen capture).

  And so, correct me if I'm wrong, but it seems to me that it's not possible to hook into GitHub’s code so that GitHub changes the state of these buttons autonomously.

[fregante]

Thanks for the research! The hook should be button[data-disable-invalid], our button does not have that: https://github.com/sindresorhus/refined-github/blob/ae247fb90a4c4b20524d76c823e97af3881326fb/source/features/one-click-pr-or-gist.tsx#L31

In this case, it should be as easy as adding the attribute

 <button
 	className={classList.join(' ')}
 	aria-label={description}
 	type="submit"
 	name={radioButton.name}
 	value={radioButton.value}
+	data-disable-invalid
 >
 	{title}
 </button>,

or maybe

+	data-disable-invalid={true}

[darkred]

Thank you!

I see that having data-disable-invalid={true} to the end, results in
Value must be omitted for boolean attributes (react/jsx-boolean-value).
And, having data-disable-invalid to the end, results in
Shorthand props must be listed before all other props (react/jsx-sort-props).

Therefore, the only option is having the former in the start: 👍

 <button
+	data-disable-invalid
 	className={classList.join(' ')}
 	aria-label={description}
 	type="submit"
 	name={radioButton.name}
 	value={radioButton.value}
 >
 	{title}
 </button>,

---

Regarding my suggested enhancement in Gists, I see that not only the 'filename' field must not be empty, but the body as well (i.e. creating empty gists is not allowed), otherwise you get the "Contents can't be empty" message error.

I tried a lot but can't make a complete working PR for this (regarding empty body).

So, maybe it would be better to just make a PR for the data-disable-invalid line, and drop my suggested enhancement for Gists? (I think it's too complicated and probably not worth it). Or wait for someone else to make a complete PR?

[fregante]

1. If GitHub allows an empty body, we allow the empty body, and vice versa. In short just add the attribute and that's it.
2. What you mentioned about the attribute is just lint stuff that you can avoid discussing, just follow the linter. My code suggestions in comments aren't meant to be 100% exact as I don't write them in my editor