Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Redesign of whole application #1069

Merged
merged 131 commits into from
Aug 24, 2023

Conversation

LukasKalbertodt
Copy link
Member

@LukasKalbertodt LukasKalbertodt commented Aug 2, 2023

This pull request implements a full redesign of Studio. This redesign was planned for quite some time and was created mainly by an external designer, Lisa, which was contracted for this, Editor, and Tobira. The redesign for Tobira was finished earlier this year; the redesign of the editor is discussed in this pull request.

We are seeking feedback from the wider Opencast community before merging this! Feel free to comment here with any thoughts, concerns or suggestions. But please read the rest of this comment before doing so!

Test the redesign here

(I try to keep this link up to date, but I might forget. If you want to check for yourself: it's the topmost link here that contains pr1069.)


Description

The main purpose of the redesign is to polish the app, improve UX here and there, but not change the users workflow completely. So for the most part, users of Studio will have to perform exactly the same clicks and actions as before. Even technical-illiterate users should not have any problems switching to this new version.
The main changes, apart from design polish, are:

  • Added a "progress bar".
  • Recording controls changed: after starting the recording, the big red button does not end the recording anymore, but only pauses it. The pause button is removed. To actually stop it, users have to press the "next" button while the recording is paused. This makes it less likely for users to accidentally stop the whole recording while trying to pause.
  • The cut markers can be positioned with mouse (or touch), not just via the buttons. The playhead can also be dragged around like that.
  • Removed seettings page. Language and color scheme settings are now directly in the header (as with Tobira). The Opencast connection settings are now integrated into the last step. This avoids an awkward redirect to the settings, and then link back. Note: for most users, connections settings are never shown as they use Studio integrated into Opencast!
  • Shortcut overview and "about" information are now modals instead of separate pages.

There are of course many more smaller changes not listed here. Further, this PR also contains a few technical changes, for example: ejecting from CRA, switch to React 18's createRoot, and replacing theme-ui with emotion.js. These changes bring Studio more in line with Tobira and Editor and tick quite a few boxes in #997.

Additional notes

There are quite a few difference compared with Lisa's design. For example, the header is quite different and my PR uses a lot less blue accent color (this was already mentioned in the Editor PR). Most of these differences have been discussed with various people and are motivated by new ideas we had or problems we encountered since Lisa finished her design. If I missed something or you have a question about a difference, feel free to just ask.

  • One feature is still missing: the voice/audio indicator in the recording step. This can be added easily in a follow-up pull request.
  • The positioning of the recording controls looks so nice and easy in Lisa's design, but is tricky in real life, where there could be two video streams, which could have weird aspect ratios, and the screen could have any size.

This PR contains almost no "breaking changes". However, the theme.customCSS settings value is not supported with this PR. As the relevant commit says, we can still add support for some color configuration in the future. But just letting organizations insert custom CSS is a very fragile solution that can break at any time. And we want to make sure that colors are only changed in a way that maintains sufficient contrasts to guarantee accessibility.

As mentioned above: the version you can test here is a "standalone Studio" where users are asked to enter the Opencast connection details. For most of you, you will probably use Studio thats included in your Opencast. In that case, those settings are not shown. Then, the last step only shows two fields for "title" and "presenter".

Known problems

I would suggest fixing those after this pull request is merged.

  • Recording on iOS fails (but it did so before as well, just differently...)
  • There are a number of screen reader issues, where the screen reader output is not ideal. I already fixed a few things in that regard, but some issues remain. Generally, accessibility should already be quite good though. So if you notice any other accessibility problems, please report!

Open questions/ideas

  • Remove settings gear when there is a stream error? The current Studio design also shows the settings gear if you disallow Studio using the camera, for example. The idea is that changing any of those settings might make the error disappear and the video stream work? But it's very likely that's actually useful. So only showing the settings gear if a working video stream exists might be better?
  • Dragging cut marker should show that frame in video preview? But once the cut marker is placed (i.e. mouseup), the play position would reset to where it was before the the drag operation started. Personally I think this is likely useful, but as it's not trivial to implement, I wanted to ask first.
  • Maybe only have one download button that downloads everything? I.e. not making it possible to only download one of the two videos?

Meta

I am on vacation next week, and on the Opencast DACH meeting in Bern the week after. So I will only respond to your comments after returning from that.

In the redesign, we won't be using a router anymore, so no need to
redirect other paths to `index.html`.
The dark mode colors are just the light mode ones in reverse. This will
still change!
We use emotion in our other projects as well and theme-ui just adds
some features to emotion that we don't really need. It also causes
certain problems.
With CRA we are forced to annotate the JSX thing in each file. It also
has lots of other problems, from slow updates, over not being able to
configure certain things, to unexpected magic behavior.

This commit just ran `npm run eject`. This left a mess which will be
cleaned up in the next commits.
This it to get them out of the way for now.
They should not stop you from viewing the app. These are things that
are sufficient to fix before committing.
I don't know how many times we already switched from one to another.
Now it's a good time to stick to the community style from the shared
ESLint config though. There are some advantages to double quotes.
This still needs accessibility features!
@LukasKalbertodt
Copy link
Member Author

@reiferschris I improved contrast some more. All text in the progress bar should have AAA contrast now. I didn't use a bolder font, but instead just made these colors generally brighter in dark mode.

I generally fixed a few things still. Everything else is something that can still be discussed after this PR (mostly bc this PR didn't even change it). My plan is to merge this in a couple hours. So if anyone still has reasons to not merge, now is the time :)

@JulianKniephoff
Copy link
Member

As announced in Tuesday's technical meeting we would love to get this in this week, still, mostly because have more cool Studio stuff in the pipeline that we would like to see unblocked. Since Lukas won't be working tomorrow that basically means today, and more specifically with regards to the comment above me that basically means now! :)

All the major things seem to be addressed, and as we also already said, this won't be the last opportunity to bring in your feedback. You are always welcome to create issues (especially for things not actually related to this PR), and there will be a design review by our external designer Lisa Steingräber courtesy of ETH towards the end of the year.

Thanks everyone for your feedback!

@lkiesow
Copy link
Contributor

lkiesow commented Aug 25, 2023

That may be why I realized that there seem to be a number of things missing. It seems odd that e.g. “Shortcuts” has a shortcut, but the buttons right next to that do not. This also goes for other places in the editor like e.g. the settings (gear) icons.

Fair. I'm happy to add more shortcuts in the future. I'm not sure all places you mention really deserve their own shortcut, but that's a discussion we can have after this PR is merged.

The problem is mostly that you introduced the shortcut overlay, which I think looks very odd in its current form. At least on the main menu.

@lkiesow
Copy link
Contributor

lkiesow commented Aug 25, 2023

@LukasKalbertodt, @JulianKniephoff: Will you create issues for the remaining open concerns raised in here?

@JulianKniephoff
Copy link
Member

@LukasKalbertodt, @JulianKniephoff: Will you create issues for the remaining open concerns raised in here?

@JasminNovotni, @lkiesow, @oas777, @reiferschris, @rrolf, @ziegenberg I hope these cover everything. Since these are not mine, feel free to double check that I didn't misrepresent your intentions. ;D

Those were the more or less concrete concerns. The rest are more open questions/things to discuss further, so here we definitely need y'all's input:

These should address everything that Lukas did not address in comments and/or additional commits. If I missed anything, or if Lukas' reactions weren't enough to adequately address someone's concern, feel free to add further issues. If you do so, it might be a good idea to refer to this PR in them and/or to link them here.

Oh, and @LukasKalbertodt should probably label these properly once he gets back! 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants