181 responsive UI

[valdelaseras]

This belongs to #181 Misc: Mobile UI

Heya CC,

This was quite an adventure :) I am well aware that this is a huge PR, so I will try to give a handy overview to help with your decision making and understanding why and what. Please do not be intimidated by the delta on first sight, a lot of these changes are ( essentially ) the same code, but moved to other places.

Changes

Code

Native web components

The most significant change. After a lot of trial and error trying to make the mobile UI work with minimal changes to the existing codebase, ultimately I decided I needed to do more rigorous refactoring to decouple and simplify the code. A big effort, but the right way to go in my opinion.

Please note that I have refactored only the code necessary to make the responsive UI work. That is something I think is important to be mindful of: there would have to be more work to turn everything into web components consistently.

• With native web components, we have encapsulated functionality per component, they are reusable, much easier to navigate and get familiar with, and much easier to build on and maintain. Everything is not tangled up as much anymore and components can easily be destroyed and reinitialised on the fly, something that was very important when considering window resize events switching from Desktop to Mobile UI etc.
• Future refactoring can follow this pattern and you can modularise the components as much as you like.
• The components in src/web/components/ prefixed with c- are replacements of the current HTMLOperation, HTMLCategory files plus a lot of functionality gathered from a variety of waiters, App and manager that could be moved to the specific components.

CSS

• you will find a lot of line changes here, but for the most part I just modularised the stylesheets further, according to SMACSS architecture. This will make it easier to navigate and work with for future contributors.

UI tests

• the existing tests have been slightly updated to work with the new UI, mostly outdated selectors etc.
• I added some mobile UI specific tests

UI changes

I have made a few UI changes that do not only apply to the mobile UI. I left some mobile features in the desktop UI as well to reduce code complexity, and I think in general these changes are good for the overall user experience anyway.

The desktop UI, operations and recipe, will look like below. io remains the same.

Favourites

• each operation has a star icon a user can click which will add the operation to favourites
• dragging an operation to favourites remains the same
• removing or updating favourites remains the same

Selected

• selected operations, either by double clicking or dragging to recipe, have a checkmark icon
• selected operations also have a slightly darker colour so it is easier for the user to see which operations are already in their recipe
• adding the same operation multiple times to recipe is still possible and the checkmark will stay visible until all of those duplicate operations are removed from recipe

The mobile and tablet UI looks like the following:

A new addition: maximised recipe pane for a comfortable user experience ( mobile / tablet only ):

Pane maximisation

• a little extra feature I added for mobile / tablet users is that they can maximise recipe, input and output panes. Mobile screens are small and finnicky and a lot of user interaction is required to use CyberChef. This way, users can have a more comfortable experience configuring their recipe, reordering, inspecting output etc.

Favourites

• as described above, operations may be added to Favourites by tapping the star icon. This was necessary as you can't add favourites by dragging when the operations list itself is scrollable, that would be a very annoying experience.

Selected

• as the categories are not visible side by side with recipe on mobile like they are on desktop, the user needs some type of feedback to know if an operation was added to recipe. Hence, the selected icon as mentioned above

Tabs

• multiple tabs on the input ( and consequently output ) component is disabled. It's too finicky on mobile in my opinion.

---

I've been working on this intermittently since April, so I hope I have not forgotten to mention any major change above. Feel free to ask if you have any questions or remarks. Take your time to test the UI yourself, I have added some configuration that allows you to easily test on your actual mobile device when you run npm start. I have put a lot of time into thinking of an intuitive UX for mobile, so I hope you will be able to use it easily on a mobile device :)

Cheers!

[bee-san]

Hey! Continuing from Discord

Can you please add the functionality of these 2 items to make it more accessible? :)

https://github.com/gchq/CyberChef/pull/1360 and this feature request https://github.com/gchq/CyberChef/pull/1409

Also on mobile if you scroll with your finger (down past the page, you cant do this on desktop but you can force scroll on mobile to the white at the bottom) some text follows you, like this text:

very minor bug for people who are forcibly scrolling through websites :)

[a3957273]

Hey @valdelaseras, apologies for taking so long to look at this truly behemoth PR. An initial comment when testing this is that it seems to break on roughly half the operations. For instance, trying to add 'entropy' creates the following error:

Uncaught TypeError: this.app.operations[name] is undefined

After this error displays, the panels are no longer draggable and no input parameters are rendered for the component. This error might have gone unseen because you can double click operations to add them still. The problem doesn't appear if you refresh the page whilst maintaining these inputs, or by entering in a new deeplink. So it seems to be something to do with the instantiation after dragging?

Some other minor things:

• There's more margin on the banner, which looks more pleasant but steals valuable screen room. I think it's probably better to stick with the same minimal margin we use in the current version.
• The 'recipe' screen on desktops starts off equally sized to the 'input'. On most desktop displays, it feels like this makes the recipe needlessly large. I think setting some sort of 'max starting size' might be reasonable, although I haven't looked into your implementation to see if that's possible.

As an extension that I wouldn't see as necessary to the PR:

• A 'tick' next to each activated operation works well when you only use one per recipe, but a lot of recipes tend to repeat the same operator multiple times. If I already have an operator in the recipe list and click it again, as a user I get no feedback that anything has happened! Perhaps if multiple operators are found, a number can be added next to the stick showing how many there are?

Initially, I was also going to suggest that the 'operation' menu should close after every new operator added, but in retrospect I could imagine that getting frustrating.

Once again, thanks for the incredible amount of work you clearly put into this. It'll likely take us several more weeks to run through all your changes.

[valdelaseras]

Hi @a3957273, thank you for your comments and observations.

I understand it takes time to work through the entire PR, no worries. Please take your time and collect any feedback you have and issues you find, then I will be sure to make time to immerse / dedicate myself into this codebase again and get this PR ready. I'm also planning to tackle the two additional requests and the bug @bee-san mentioned above at that time.

The UI remarks you mentioned seem easy enough to update. I've also considered a counter for 'duplicate' Operations, but was a little unsure if it was necessary, as you have to remove any individual Operations from the Recipe manually (the user would see how many duplicates there are). However, I agree that it would be better if there was some way to see that from the dropdown itself and to have visual feedback of the Operation being added as a duplicate. I don't think it will be very hard to add a little counter next to the checkmark.

I do not recall the error but I'm confident I'll be able to fix it. Thank you for reporting that.

I'm looking forward to your further feedback! Cheers

[a3957273]

Wow, this PR is really fantastic! Whilst I'm cautious about the "more lines of code equals less comments" meme, there really are very few comments I have on improving this.

You're absolutely correct in your first prompt that a lot of these changes are actually just moving things around, and most of the rest are adopting native web components. Both these changes would be fantastic alone, let alone that this introduces a far superior mobile UI.

In general, if you manage to solve the:

Uncaught TypeError: this.app.operations[name] is undefined

Error that occurs with some operations I'd be happy to get this merged in. 👍

Thanks once again for the work you've put into this.

[valdelaseras]

Thank you a3957273, I'll make time soon :)

[a3957273]

Took us long enough to review it, so no hurries. Just whenever you get around to updating it!

[valdelaseras]

@a3957273 sorry, could you please describe how you're getting the error? I can't seem to reproduce it, operations are added normally which makes it a little tricky to debug this. Perhaps it's the drag event specifically or something like that. Any additional info would be super helpful :)

[a3957273]

Exciting to see a fix for the undefined operation name. Would you similarly be able to resolve the conflicts with the main branch? I'll hold off on merging any major changes to main branch for two weeks to ensure there aren't any more conflicts.

[valdelaseras]

Hi @a3957273, ~the undefined error should be resolved with the last commit. Are you still seeing it?~ Sorry, misread that :)

Yeah, I've been in the process of solving the merge conflicts. Its going to take a little bit of refactoring unfortunately, as the changes were made on files that don't exist anymore in this branch. Thanks for letting me know about the time-frame