Why not just do it in this same one?
It’s better to avoid doing several things in one PR. Small PRs are easier to follow, it’s not only important now when we’re reviewing this but also will be important in the future when we’ll be revisiting the history. But more importantly - this change can cause issues on specific platforms and browsers. The smaller the change - the less a chance I miss something when testing this or reviewers miss some important detail.
There are two tasks here:
- Get rid of the custom modal
- Start passing supported file extensions to the system modal (we weren’t doing it before)
By the way, these tasks can even be done in the opposite order.
I think it is a bit of a confusing experience at the moment
Fair point, but at the same time I think it’ll be confusing only for a tiny amount of users. Most of them just always upload allowed types of files (images and so on). If someone tries to upload an unsupported file he would get the modal with a clear explanation of what’s wrong.
I think it’s more like an annoying experience, not a confusing one. It’s not like “I don’t understand what’s going on”, it’s more like “Why didn’t you tell me earlier that this type of file is unsupported?”
So I’d like us to have every step in a separate PR, the second PR is going to be tiny and will follow soon. @martin-brennan what do you think?