Editor: Exposed supported formats in Loader.js;

[ycw]

Why refactor:

Editor supports two ways of importing files from fs: accept does guard users from selecting unsupported files in the first place for File>Import, however there's no equivalent facility for the 2nd way, drag n drop, so DIY UI feedback is needed for notifying users that no-op as they dropped unsupported files, I prefer showing Import Error: Unsupported file format (xxx) together with editor's supported file formats (same purpose of accept that supported file formats are listed in browser file dialog), my attempt was using #28295 and #28295 (comment), so 'introspection of supported formats' is inevitable, that's why file handlers are re-organized to help introspect in getSupportedFileFormats().

The PR:

• Extracted file handlers into fileHandlers object:

  • The handler interface is (editor, manager, reader, file, ...handlerSpecificOptions): void
  • This object is for internal used
  • To support new file format, simply adds a handler like fileHandlers[ 'myFormat' ] = myFormatHandler
  • To remove support of a file format, simply deletes it from fileHandlers

• Exposed getSupportedFileFormats():

  • This is a static method
  • The supported file formats are ... auto-derived ... from fileHandlers object
  • Returns array of extension names; sorted by sort()
  • One of the consumer is MenuBar.File.js: Use getSupportedFileFormats() to generate accepts list for File>Import input (not included in this PR)
  • Solved Editor: Alerts if File>Import failed #28295 (comment)

[mrdoob]

Hmm, doesn't it make the code less readable?

IMHO maintaining the input.accept list doesn't seem too bad because the file browser wouldn't let the files be loaded if not updated.

[ycw]

IMHO maintaining the input.accept list doesn't seem too bad

see #28295 (comment)

[mrdoob]

Hmm, doesn't it make the code less readable?

IMHO maintaining the input.accept list doesn't seem too bad because the file browser wouldn't let the files be loaded if not updated.

@Mugen87 what do you think?

[ycw]

In short, this PR can be closed if ... hardcoded supported file formats is accepted for bith file input `accept` value, and the supported file formats hint in UI feedback :D, Expand it to see why refactoring is needed, thanks.

---

Why refactor:

Editor supports two ways of importing files from fs: accept does guard users from selecting unsupported files in the first place for File>Import, however there's no equivalent facility for the 2nd way, drag n drop, so DIY UI feedback is needed for notifying users that no-op as they dropped unsupported files, I prefer showing Import Error: Unsupported file format (xxx) together with editor's supported file formats (same purpose of accept that supported file formats are listed in browser file dialog), my attempt was using hardcoded supported formats list here and was rejected, so 'introspection of supported formats' is inevitable, that's why file handlers are re-organized to help introspect in getSupportedFileFormats().

[Mugen87]

Fair enough! Since there is now more than one use case that uses the file format list I'm okay with adding it.

[Mugen87]

Do you mind resolving the merge conflicts as well?

[ycw]

done

[mrdoob]

As far as I understand, the main purpose of this PR was to enable Loader.getSupportedFileFormats().

Now that #28396 has made the method no longer needed, should we revert this PR?

[Mugen87]

TBH, there is no reason anymore for the refactoring. If you prefer the previous code style, I'm fine with reverting, too.

[mrdoob]

What code style do you prefer?

[Mugen87]

The previous one, tbh. If I would implement the module from scratch, I would rather use a switch/case statement like before than storing the handlers in an object.

[mrdoob]

@ycw

We appreciate all the improvements you're doing to the editor.

However, it may be better to share what you're planing to work on before spending a lot of time working on a PR that changes a lot of files or refactors code.

If you do that, we may be able to help you on coming up with simpler solutions and save you time.

[ycw]

We appreciate all the improvements you're doing to the editor.

Thanks.

it may be better to share what you're planing to work before spending a lot of time working on a PR

Had been in #28324 (comment) I should write it at the OP sorry for that 🙏🏻

that #28396 has made the method no longer needed, should we revert this PR?

This PR was not originally for #28396, instead it was for #28295, (I even added 'TODO: UI Feedback' in commit :D), so no.