@Prtksxna Could you update this ticket with the design once you've fixed the accessibility issues and made it more prominent?

A quick question about your comment on our doc — we'll need the upload progress state to completely block all other interactions, right?

I was imagining that the upload process would be a standard form-submission, i.e. that the browser would indicate progress, and that navigating away before it'd finished (not that it'll take very long, hopefully) would abort the process.

@Prtksxna Yep - no other interactions while upload is in progress makes sense. We can change the text for the "Upload to Commons" button to be "Uploading to Commons..." (or similar) with the diagonal-bars animation to indicate upload in progress. Doing a progress bar is nicer but more complex too.

If the user navigates away before upload finishes, then we abort the upload process. In future we might think about giving the user a dialog when they try to navigate away before upload finishes but it's not a part of this ticket.

In T210564#4836095, @Samwilson wrote:

I was imagining that the upload process would be a standard form-submission, i.e. that the browser would indicate progress, and that navigating away before it'd finished (not that it'll take very long, hopefully) would abort the process.

…or…

In T210564#4848072, @Niharika wrote:

@Prtksxna Yep - no other interactions while upload is in progress makes sense. We can change the text for the "Upload to Commons" button to be "Uploading to Commons..." (or similar) with the diagonal-bars animation to indicate upload in progress. Doing a progress bar is nicer but more complex too.

If the user navigates away before upload finishes, then we abort the upload process. In future we might think about giving the user a dialog when they try to navigate away before upload finishes but it's not a part of this ticket.

I think both these options make sense.

@Niharika, I don't think we have a button with that state in OOUI yet but I can add designs if we'd be willing to implement. Also we'll should temporarily disable all the translation text fields on the page just so that the user doesn't make changes that wont be submitted.

Oh. I misread what Sam was suggesting. I think it's okay for the browser to indicate progress. We don't need the button-animation but I do think we want to change the button text (and color?). Disabling the translation fields makes sense.

@Samwilson Note that the banner image is only for design purposes and the text that will go in it is specified below the image. Also, this ticket assumes that the user never has to leave the translation page i.e. the upload happens behind the scenes. Let me know if that is not the case and I can revise the spec as needed.

They won't leave the translation page, but the page will reload. Is that okay? It means we don't have to worry about keeping other state in sync and everything will (after the upload) be in the most up-to-date state possible. And we get the browser's activity indicator.

The ToolforgeBundle currently only authenticates to Meta, so before this ticket is done I think we should modify the bundle to support any wiki so that this can be tested against other upload destinations.

Is that a parameter of the OAuth permissions we request or is it something more?

No, the permissions can be set per-consumer, so that bit is okay.

I think the only thing we need to do is update this line to accept an environment variable — but I'm not quite sure where we handle the default (which I guess should remain as Meta… although I sort of wonder if leaving it for each app to set might not be a good idea too, to avoid errors like me attempting to upload 34 test files to Commons).

I'm attempting a patch to the bundle now, so will know soon if it's going to be a hassle or not.

This should fix the bundle: https://github.com/wikimedia/ToolforgeBundle/pull/10

In T210564#4897067, @Samwilson wrote:

They won't leave the translation page, but the page will reload. Is that okay?

Yep, that's fine.

Now the bundle can talk to any wiki, the next stumbling block we've got is that the mediawiki/oauthclient library that we use only supports file uploads in its latest master. So we need a new release made for this before we can use it (see T214440). Once that's done we can add a little file-uploading wrapper to the bundle, and we'll be on our way.

Is there a working prototype of the banner?

The greens look like green90 and green30 for the background and border of the banner, but I'm not sure what the buttons should be. Maybe they're the border green with an opacity?

Are all corners of radius 3px?

Should the banner be the same width as the header/tutorial/footer, i.e. 950px max?

In order to test the upload functionality, we'll want to point the staging site to https://commons.wikimedia.beta.wmflabs.org/

https://github.com/wikimedia/svgtranslate/pull/57 makes this possible. It's ready for review.

PR57 is merged and deployed. Now the staging site connects to beta Meta for authorisation and to beta Commons for image-fetching. Search seems weird though. :-(

https://github.com/wikimedia/svgtranslate/pull/56 is the remaining work for this ticket.

I think there are some bugs around how we're writing translations into the SVG, but I suggest that these are fixed in follow-up patches in order to keep this one focused on the uploading. (For example, it seems that when we write in an incomplete translation we copy the fallback strings for the missing text, which is bad.)

In T210564#4904238, @Samwilson wrote:

Is there a working prototype of the banner?

The greens look like green90 and green30 for the background and border of the banner, but I'm not sure what the buttons should be. Maybe they're the border green with an opacity?

Are all corners of radius 3px?

Should the banner be the same width as the header/tutorial/footer, i.e. 950px max?

Ping @Prtksxna. I know there was a working prototype but last I checked, Prateek's prototype was down. Prateek, can you confirm the above details?

In T210564#4907281, @Samwilson wrote:

In order to test the upload functionality, we'll want to point the staging site to https://commons.wikimedia.beta.wmflabs.org/

https://github.com/wikimedia/svgtranslate/pull/57 makes this possible. It's ready for review.

Oh, awesome! I didn't realize we had a commons beta site. This is great!

In T210564#4908086, @Samwilson wrote:

PR57 is merged and deployed. Now the staging site connects to beta Meta for authorisation and to beta Commons for image-fetching. Search seems weird though. :-(

https://github.com/wikimedia/svgtranslate/pull/56 is the remaining work for this ticket.

I think there are some bugs around how we're writing translations into the SVG, but I suggest that these are fixed in follow-up patches in order to keep this one focused on the uploading. (For example, it seems that when we write in an incomplete translation we copy the fallback strings for the missing text, which is bad.)

@Samwilson It'd be great if you can file tickets for these bugs. :)

Oh, awesome! I didn't realize we had a commons beta site. This is great!

There seems to be an issue with search on Beta Commons, but it's working okay for older files. I made a comment about it on T186993 (but that might not be the right place).

It'd be great if you can file tickets for these bugs. :)

Done: T214717

In T210564#4904238, @Samwilson wrote:

Is there a working prototype of the banner?

There is, but I am not able to figure out why Github is refusing to serve the assets. If you could download the prototype and run an HTTP server you can see the banner after clicking Upload to Commons.

Patch merged, and staging site updated.

Waiting on testing this more thoroughly once T215187: [BUG] Resolve issues with broken image rendering on SVG Translate is fixed.

Note - this patch is live on production instance on svgtranslate. I accidentally uploaded a new version of https://commons.wikimedia.org/wiki/File:100_Years_War_France_1435-sr.svg as I was playing around.

@Samwilson Was testing this and have a few follow-up things:

1. The upload comment doesn't match the one in the task description.

• Upload the SVG to Commons as a new version of the previous file. The file is uploaded on behalf of the translation author.
  • Upload comment - File uploaded using svgtranslate tool (https://tools.wmflabs.org/svgtranslate/). Added translation for <langcode>.

2. The "Upload to Commons" button does not change color or get disabled during upload

• During the upload:
• The Upload to Commons button changes from blue to gray, becomes disabled and the text changes to Uploading to Commons.... Reverts back to original state after upload completes.

3. If a user hits the "Upload to Commons" button without changing anything - that is, the file is exactly same as the one on Commons, they get a 500 error. We should not do anything at all if the file is the same on Commons. Keep the user on the same page.@Samwilson should I file this as a separate ticket?

1. https://github.com/wikimedia/svgtranslate/pull/65
2. https://github.com/wikimedia/svgtranslate/pull/66
3. I was going to say that that should be separate, but I was thinking that we'd have to be keeping track of their unsaved changes... but of course in that situation there are no unsaved changes. Perhaps the solution is to prohibit upload (or download) when the form is 'unsaved' (which we're already tracking for the don't-leave-the-page alert, so it shouldn't be too hard to add). And of course, we should also show a nicer error page than the uninformative 500; I thought there was already a ticket for that (i.e. we want to show every app error in a nicer way), but I can't find it.

PR for point 3 above is: https://github.com/wikimedia/svgtranslate/pull/67

@Samwilson Please update prod with the merged PRs above. Especially the one with improved upload comment.

Doing so will also deploy T214452; is that okay?

In T210564#4949871, @Samwilson wrote:

Doing so will also deploy T214452; is that okay?

Yes.

Production site updated to 0.2.0.

The last bit of this task is https://github.com/wikimedia/svgtranslate/pull/67

As discussed in today's standup, I'll remove the arrow in the link.

This is now ready for QA.