-
-
Notifications
You must be signed in to change notification settings - Fork 226
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
Added Profile selection and a Standby Clock #1829
base: main
Are you sure you want to change the base?
Conversation
The Pull Request contains a new profile selections screen. Users can click the printer name and available profiles, created in OctoPi will be populated. Users can click on profile and hit the select button to switch the profile. The action will disconnect the current connection and re-connect with a new profile. A check is made for an ideal printer i.e. the profile selection screen will only populate when the printer is not doing any print job. A clock is added to the stand-by screen with some animation to save the screen. |
I'll have a look at this soon :) Thanks for rebasing! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. There are some major changes that need to be addressed before this can be merged. The three main things are:
- Duplicate css code
- Please try adhering to the TypeScript Style Guide: https://google.github.io/styleguide/tsguide.html (or use Prettier to format your code)
- No screenshots attached. If you create a UI PR please attach them to make the review easier
I haven't run the code yet, so I can't comment on the UI and the current remarks are just from a quick brushover. Once those have been addressed we can do a full review and after that this can be merged.
Thank you for your contribution! In the future please separate different features into different Pull Requests though, this makes the review process a lot faster & easier :)
<img src="assets/fr.svg" class="level_icon" /> | ||
</div> | ||
</div> | ||
<span class="homing-text" *ngIf="isHomingNeeded">Homing is needed before starting leveling.</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<span class="homing-text" *ngIf="isHomingNeeded">Homing is needed before starting leveling.</span> | |
<span class="homing-text" *ngIf="isHomingNeeded">Homing is necessary before leveling.</span> |
What do you think about this?
width: 100%; | ||
height: 100%; | ||
background-color: rgba(0, 0, 0, 0.85); | ||
// backdrop-filter: blur(8px); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove comment
&__sub-heading { | ||
font-size: 2.3vw; | ||
font-weight: 500; | ||
padding: 2vh 0 1vh; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove all classes that aren't used in bed-leveling.component.html
from this file.
public fadeOutAnimation = false; | ||
public isHomingNeeded = true; | ||
private currentProfile: PrinterProfile; | ||
private axisOffset: number = 15; | ||
@Output() closeFunction = new EventEmitter<void>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public fadeOutAnimation = false; | |
public isHomingNeeded = true; | |
private currentProfile: PrinterProfile; | |
private axisOffset: number = 15; | |
@Output() closeFunction = new EventEmitter<void>(); | |
public fadeOutAnimation = false; | |
public isHomingNeeded = true; | |
private currentProfile: PrinterProfile; | |
private axisOffset: number = 15; | |
@Output() closeFunction = new EventEmitter<void>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
axisOffset also should be configurable. But i can do that if you like.
profileService.getProfiles().subscribe((profiles) =>{ | ||
for( let profile of profiles){ | ||
if (profile.current) { | ||
this.currentProfile = profile; | ||
break; | ||
} | ||
}; | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be part of ngOnInit rather than the constructor
@@ -111,3 +133,131 @@ app-custom-actions { | |||
opacity: 1; | |||
} | |||
} | |||
|
|||
@import url("https://fonts.googleapis.com/css?family=Exo:400,700"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need another font here?
@@ -17,13 +17,37 @@ export class StandbyComponent implements OnInit, OnDestroy { | |||
public showConnectionError = false; | |||
private displaySleepTimeout: ReturnType<typeof setTimeout>; | |||
private connectErrorTimeout: ReturnType<typeof setTimeout>; | |||
|
|||
title = 'clock-greets'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this isn't used anywhere
time; | ||
hours; | ||
msg; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add access modifiers and type here
setInterval(() => { | ||
this.time = new Date(); | ||
this.decide(); | ||
}, 1000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this interval is never cancelled and 10s timeout probably will be sufficient.
<svg viewBox="0 0 100 100" y="0" x="0" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"> | ||
<path stroke-miterlimit="10" stroke-width="3.5" stroke="#f5f6fa" fill="none" d="M84.84 90H15.16A5.16 5.16 0 0 1 10 84.84V15.16A5.16 5.16 0 0 1 15.16 10h69.68A5.16 5.16 0 0 1 90 15.16v69.68A5.16 5.16 0 0 1 84.84 90z" style="stroke:#f5f6fa" ></path> | ||
<circle fill="#f5f6fa" r="8.504" cy="67.006" cx="32.992" style="fill:#f5f6fa;animation-play-state:paused" ></circle> | ||
</svg> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you create those icons yourself? If not please make sure to link them according to the LICENSE.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Hi! I don't want to pressure you into anything, just curious, whether you want to continue working on this PR? If not I would take this over and pull a few features into main in separate PRs. Just let me know if you aren't fine with that. Thanks! |
Hi, for past couple of weeks my day job is asking much of my time and I
couldn’t get much for this or another branch (Wifi configuration via
hotspot) I was working on.
It is fine if you want to take over this.
I might get free after this weekend .
Regards,
Priyank
On Wed, Jun 9, 2021 at 4:41 PM Timon G. ***@***.***> wrote:
Hi!
I don't want to pressure you into anything, just curious, whether you want
to continue working on this PR? If not I would take this over and pull a
few features into main in separate PRs. Just let me know if you aren't fine
with that.
Thanks!
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1829 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AQMYX7NL3HBZFLX27VLKYBDTR7NYJANCNFSM45GB4K4A>
.
--
Maker's Cave
https://makers-cave.com <https://www.makers-cave.com/>
https://fb.me/makerscave.us
(If you like our service, please like us on Facebook.)
|
Discard the previous pull requests. rebased the source with v2.2.0 and merged my changes