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

UI Overhaul #1552

Merged
merged 45 commits into from
Sep 27, 2021
Merged

UI Overhaul #1552

merged 45 commits into from
Sep 27, 2021

Conversation

pajlow
Copy link
Contributor

@pajlow pajlow commented Nov 18, 2020

Hello, so this is my progress so far on #1150 . This PR gives an overview and is by far not the fully working installer. The next step would be to migrate all modals to svelte and then continue to handle all data exchanged between the components and the node backend properly. Just let me know if I should continue and if there are things you would handle differently. ATM the code is not very organized because I still have to implement a lot of things and clean up afterwards. After all dependencies are installed npm run dev is enough to get you started.

@NeoTheThird
Copy link
Member

Weee, i'm so looking forward to playing with this. Thank you so much, i'll review asap and reserve 0.9.0-beta for hopefully rolling this out. :)

Copy link
Member

@NeoTheThird NeoTheThird left a comment

Choose a reason for hiding this comment

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

I don't think we need public/favicon.png, do we?

@pajlow
Copy link
Contributor Author

pajlow commented Nov 18, 2020

No, it is basically just leftovers from the official svelte template. And there is still a lot of duplicate assets in there which will be removed during further progress.

Copy link
Member

@NeoTheThird NeoTheThird left a comment

Choose a reason for hiding this comment

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

Very cool. Here are some things that jumped at me right away, but definitely on a good path, great work!

The duplicated files are due to the remaining pug components, i assume?

You might want to run npm run lint-fix in your repo to enforce the code style.

public/index.html Outdated Show resolved Hide resolved
rollup.config.js Outdated Show resolved Hide resolved
src/App.svelte Outdated Show resolved Hide resolved
src/main.js Outdated Show resolved Hide resolved
src/main.js Outdated Show resolved Hide resolved
@NeoTheThird
Copy link
Member

As for the conflicts, i would wait for #1553 to be merged and then rebase this branch on master. The gitignore conflict can be resolved by accepting both changes. Resolving package.json will need some brains to ensure consistency in the scripts and dependencies. After package.json is resolved, package-lock.json can finally be deleted, re-built using npm install, and re-committed.

@NeoTheThird
Copy link
Member

I sent you a PR to resolve the conflicts, if that's ok with you. I hope i did not break any of your changes. https://github.com/pajlow/ubports-installer/pull/1

Great work so far! I like the new state-aware way of handling modals and views. Still some stuff to be done, i think, but it looks to me as if this is on a very good way. How do you want to handle this further, are you planning to continue to work on it? Or do you want someone else to take over? What's your strategy regarding CSS? I'm also not afraid of properly changing how some stuff in the UI works, so if you feel like it's too difficult to replicate the old behavior in some parts, i'm open for new ideas.

@pajlow
Copy link
Contributor Author

pajlow commented Nov 20, 2020

That is fine. I will merge it today. I just saw you deleted one development related command/dependency which I use for auto reload in svelte.
I think I can get a lot of stuff done over the weekend and next week. By css you mean Bootstrap and overall styling? Bootstrap is bundled in theme.css inside the public/build folder and there is also one global css file for some things. Everything else I would handle inside the components.
Thinks I can't promise that they will work are some of the animations. I plan to get everything running and try to implement them afterwards.

@pajlow
Copy link
Contributor Author

pajlow commented Nov 24, 2020

@NeoTheThird What are the advantages of changing svelte.js and rollup.config.js to .mjs files? They broke some things for me when including require. Do you have another solution for implementing reload when developing?

@NeoTheThird
Copy link
Member

NeoTheThird commented Nov 24, 2020

@pajlow you were using es6 import in those files, so they need to be modules. If you want to use commonjs require, feel free to change it back. Make sure to check the linter (npm run lint / npm run lint-fix)

@pajlow
Copy link
Contributor Author

pajlow commented Nov 26, 2020

@NeoTheThird is unlock modal the follow up to oem-lock? I have unfortunately no locked device to test it...

@NeoTheThird
Copy link
Member

@NeoTheThird is unlock modal the follow up to oem-lock? I have unfortunately no locked device to test it...

@pajlow no, it's used to display instructions before the os selection screen. the old canonical devices, for example, need to not be running android, before the installer can do anything. you can just select the bq aquaris e4.5 in the device selection to see what this looks like. btw, if you want to bring up any of the modals in the pug version, start the installer with --debug to bring up the debugger and enter modals.show("oem-lock") in the web console to see the modals.

@pajlow
Copy link
Contributor Author

pajlow commented Nov 30, 2020

@NeoTheThird I am almost done with the views. Next up will be the remaining modals but I have to rewrite some stuff because there is a lot of dom element creation going on there.

@NeoTheThird
Copy link
Member

Awesome! So happy to finally get rid of the dom creation, that should be so much simpler now!

Would you mind if we already merge #1576? That one touches a lot of the backend files and also a few frontend things, but I'm happy to help resolve the conflicts, of course.

@pajlow
Copy link
Contributor Author

pajlow commented Dec 1, 2020

@NeoTheThird No problem. I will merge it this evening after you open the PR.

@NeoTheThird
Copy link
Member

alright, all done, won't touch the pug files from now on :) rebase away

@NeoTheThird
Copy link
Member

Alright, i sent you a PR here https://github.com/pajlow/ubports-installer/pull/2. That should handle most of the cleanup. make sure you read my note on the linter. you should be able to just rebase-and-merge that pr for the commits to show up here and then pull (use git pull --rebase to be sure, if you have local commits) your local branch to have the changes.

pajlow and others added 2 commits March 11, 2021 20:08
Co-authored-by: Jan Sprinz <NeoTheThird@users.noreply.github.com>
@NeoTheThird
Copy link
Member

Don't forget to run npm run lint (and commit the result) once you're ready to have the svelte files styled. How much time are you calculating on the remaining issues? Thinking if we should do another release with small backend fixes before this, or if we should wait to avoid further conflicts. No pressure either way, tell me what's best for you.

@pajlow
Copy link
Contributor Author

pajlow commented Mar 12, 2021

The linting is working perfectly, thanks! So far I resolved the udev modal issue and added Yumi. I think I will still need some time to go through the files and adjust the styling. Just go ahead with another release. I'm okay with resolving some conflicts.

@pajlow
Copy link
Contributor Author

pajlow commented Mar 16, 2021

Remaining things are animations(particles, push files and download), accordion in dev mode modal (I used cards, because there is no component based solution for bootstrap 5 in Svelte yet). I can't check all modals because I can't mock all different scenarios. Is the overall installation process working? I have only one device I use as a daily driver.

@Vince1171 Vince1171 mentioned this pull request Mar 22, 2021
@pajlow pajlow requested a review from NeoTheThird March 29, 2021 20:22
@NeoTheThird
Copy link
Member

Hey @pajlow, thanks again for your work, and apologies again for the long pause. I would now like to move forward with this. I think most of your work here is done, if you want to, i can take it from here. To allow me to merge this PR, you would have to accept and merge https://github.com/pajlow/ubports-installer/pull/3. If you don't have time for that right now, that's also fine, then i will merge your contributions into our master directly. Whatever works better for you :)

@pajlow
Copy link
Contributor Author

pajlow commented Sep 26, 2021

Hi, welcome back! I am okay with you taking over. Did the current state of my repo work for you? I had some trouble with the remote.getGlobal calls.
I would like to continue to contribute after the merge. Further things to consider are the now stable bootstrap 5 and Sveltestrap which offers bootstrap elements as svelte components. :)

@NeoTheThird
Copy link
Member

Thanks! No, it did not work completely, but i managed to get through in the dev tools with some trickery. But that's fine for now. My suggestion would be to merge this as-is and break some stuff, then incrementally fix things until everything works again and shoot for an 0.9 series release soon-ish. It's a good idea to try and move fast here, i think, because these constant conflict resolutions eat up too many resources. And if we need a hotfix on one of the 0.8 series releases, we can still do that by branching off an older state, we don't need to keep master pristine for that.

Alright. Do you maybe just want to merge https://github.com/pajlow/ubports-installer/pull/3, then i can use your PR here, that way your name is on the PR, too, and not just on the commits.

I would like to continue to contribute after the merge. Further things to consider are the now stable bootstrap 5 and Sveltestrap which offers bootstrap elements as svelte components. :)

Sounds great, looking forward to that!

@pajlow
Copy link
Contributor Author

pajlow commented Sep 26, 2021

Merged!

Copy link
Member

@NeoTheThird NeoTheThird left a comment

Choose a reason for hiding this comment

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

🚀

@NeoTheThird NeoTheThird merged commit d3b0d7f into ubports:master Sep 27, 2021
@NeoTheThird
Copy link
Member

I am very happy with this! After some fixing and intensive testing, i think we are very close to having everything working again. Great work! Really looking forward to doing more with this. This is going to unblock so many UI-related things! 🎉 🎉 🎉

@capsia37
Copy link

capsia37 commented Oct 5, 2021

Hello! I've been looking at this PR for some time and I'm very happy that its development continued. I've made some designs for the installer some time ago and I'd like to share them in case they are useful for future improvements.

https://www.figma.com/file/aHMB2HNuWn0ilqSq3L0O0A/Installer

Thank you for all your work :)

@pajlow
Copy link
Contributor Author

pajlow commented Mar 19, 2022

@capsia37 maybe you should open a new issue to get some thoughts on your designs. :)

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.

None yet

3 participants