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

Web UI migration to Vite and Vue3 and improvements to the UX #1673

Merged
merged 93 commits into from
Dec 28, 2023

Conversation

TheElixZammuto
Copy link
Contributor

@TheElixZammuto TheElixZammuto commented Sep 24, 2023

TO-DO

  • Add Vite Build step (You can build the html assets by simply running npx vite build after an npm install)
  • Update Docs
  • Test the Configuration Page
  • handle the SRC_ASSETS from CMake
  • Packaging (folders are now correctly places, must be tested on all combinations)

Description

This PR is a big one, but the biggest changes couldn't be split and I did the smallest while fixing the biggest:

  1. Use Vite as a build system for building and bundling the assets, instead of using directly node_modules. This allows us to use npm dependencies like ES modules, and to simplify the deployment of the static assets.

  2. Moved the bundling of the HTML header and content from Sunshine to Vite, using EJS. This allows us to self-host the UI without Sunshine, for testing (we can deploy the UI on GitHub pages e.g.) and for helping with tralsation tools. The shared HTML components (like the header) are now Vue Single File Components

  3. Migrated Vue from version 2 to version 3, with all the breaking changes done

  4. A revamped welcome page, with links to the support and legal directly in the welcome page

  5. Minor fixes to the applications page: now detatched command lists behave in the same way as the do-undo commands

  6. Minor styling changes to the app and PIN pages

Screenshot

welcome
apps_1
apps_2

Issues Fixed or Closed

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Dependency update (updates to dependencies)
  • Documentation update (changes to documentation)
  • Repository update (changes to repository files, e.g. .github/...)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated the in code docstring/documentation-blocks for new or existing methods/components

Branch Updates

LizardByte requires that branches be up-to-date before merging. This means that after any PR is merged, this branch
must be updated before it can be merged. You must also
Allow edits from maintainers.

  • I want maintainers to keep my branch updated

@TheElixZammuto TheElixZammuto marked this pull request as draft September 24, 2023 20:29
@Nonary
Copy link
Collaborator

Nonary commented Sep 28, 2023

I'll take a look at this over the weekend, I am a full stack dev and have experience in Vue so if you need an extra pair of eyes for a code review I'm down for that.

Copy link
Member

@ReenigneArcher ReenigneArcher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to pinning dependencies, can you also ensure new lines exist at the end of files?

I haven't dug into the rest of it yet. Would be nice to incorporate #972 into this or a separate PR shortly after this one. The dev of that one hasn't been active, probably easier to recreate their work than rebase the PR now.

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Copy link
Member

@ReenigneArcher ReenigneArcher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to the build comments. Here are a few related to the UI.

Can you add some padding at the bottom of the home page? If it matched the troubleshooting page, I think that would be fine.
image

On Applications, the "edit" button doesn't work, unless the lower portion is already expanded with the "add new" button
image

The "x" does not work for the cover finder. Actually, neither does selecting an image. (Clicking off of the modal does close it though)
image

Add Detached Command button does not work.

Save application does not work for existing applications. I added "test.log" to output, saved, and then edited again, but the field remained blank. Seems to work for new applications.

Configuration seems a little wonky. There is a square around some options, but not others.
image

Moving to the "Files" tab, the options from general that were not in the box, appear at the top of "Files".
image

And the files options appear at the bottom in a "cutoff" box.
image

That behavior continues for the remainder of the tabs. (The non boxed options from "General" appear at the top of each tab.

I did not test any of the buttons in the Configuration section.

Troubleshooting/Logs... can be a separate PR, but could the "Find" feature ignore the case?

I am using Firefox, I don't know if that has any impact on anything. I reloaded the UI with no cache as well.

Lastly, the tray icon has no image.
image

Tested on Windows 11.

docker/archlinux.dockerfile Outdated Show resolved Hide resolved
docker/debian-bookworm.dockerfile Outdated Show resolved Hide resolved
docker/debian-bookworm.dockerfile Outdated Show resolved Hide resolved
docker/fedora-38.dockerfile Outdated Show resolved Hide resolved
docker/ubuntu-22.04.dockerfile Outdated Show resolved Hide resolved
@TheElixZammuto
Copy link
Contributor Author

Thank you, I've checked everything (paddings, configration page issues, app page issues, missing tray icon) and everything should now be in order

@ReenigneArcher
Copy link
Member

One more observation:

The delete button doesn't work for command preparations
image

I think everything else is good to go.

@ReenigneArcher
Copy link
Member

@TheElixZammuto not sure if you saw my last review. #1673 (comment)

That's the only thing I saw not working. It's possible it doesn't work in the current version as well (I didn't check).

@TheElixZammuto
Copy link
Contributor Author

Fixed that (it already worked on app list, only global commands did not work)

@ReenigneArcher ReenigneArcher merged commit 5bdbda9 into LizardByte:nightly Dec 28, 2023
41 checks passed
KuleRucket pushed a commit to KuleRucket/Sunshine that referenced this pull request Jun 6, 2024
e-dong pushed a commit to e-dong/Sunshine that referenced this pull request Jul 26, 2024
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.

3 participants