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

Discuss a new pattern for the Web UI #773

Closed
wants to merge 9 commits into from

Conversation

bkjohnson
Copy link

Summary

As the title implies, I'm not trying to get this PR merged in its current state - parts of it aren't that great and it wouldn't even be complete. Instead, I wanted to use it as a way to talk about a different structure for the Web UI which I think would be helpful.

This PR co-locates styling, markup and behavior in the same place (without adding any dependencies) by using more modern JS standards and APIs. The main feature on display is custom elements, which I've used to create a <stats-dialog> element which then wraps a <dialog-component> element.

Problems with current Web UI

In its current state, the Web UI places all the markup in the index.html file, and then there are about a dozen different site-specific JS files, some of which perform DOM modification using JQuery assuming the existence of elements in index.html. I don't think that this structure would be easy to maintain in the future if features were to be added, since a change could introduce an issue by breaking a DOM node that one of the many JS files is expecting to be there. JS modules and scoping the markup to a custom element would make this easy to avoid.

CSS

There is the additional issue of global CSS, which can make any styling changes frustrating and difficult. Web Components use the shadow DOM, so styles defined inside the component don't leak out to affect other elements on the page, but the custom elements can still allow themselves to be styled by site-wide CSS or by parent components through the use of CSS variables. I haven't demonstrated these here, but if used appropriately they can be quite useful at creating style-agnostic components without relying on things like !important which creates additional problems to deal with in the future.

Browser Support

With new things like this browser support is obviously a concern, but I've been careful to make sure that these changes will work in modern versions of Chrome, Firefox, and Safari. I've locally tested the changes on Firefox and Chromium and encountered no missing functionality.


If this is something that people would be interested in pursuing, I would be happy to help out. I'm also interested in hearing what people have to say about this idea.

This is just styling & layout at the moment,
more will be added later.
This commit also removes the old HTML which was
displaying the statistics & the code for fetching
the data is also refactored to use promise chaining
and also not rely on JQuery
This makes <dialog-component> a bit less flexible, but
it should make it easier & more consistent to use
@mikedld
Copy link
Member

mikedld commented Dec 2, 2018

Transmission is a software used and relied on by a lot of people, not a playground for new and shiny technologies that are still being worked on. If you want to modularize the web UI code, more sensible approach at this time would be to e.g. help out with fcsonline/react-transmission which does basically the same thing while supporting more browsers and browser versions.

@bkjohnson
Copy link
Author

I don't view transmission as a playground, but thank you for the feedback.

@ckerr
Copy link
Member

ckerr commented Oct 12, 2021

Is this PR still germane after the web client refresh that landed in #1476?

I believe most of the site-specific code is now gone, as is jQuery and jQueryUI. However it does not use custom elements.

@ckerr ckerr added scope:web needs clarification More info is needed before work can proceed labels Oct 12, 2021
@bkjohnson
Copy link
Author

I'm not sure since I haven't used the web UI for quite a while. I'll close.

@bkjohnson bkjohnson closed this Oct 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs clarification More info is needed before work can proceed scope:web
Development

Successfully merging this pull request may close these issues.

None yet

3 participants