Issues #16 and #19

[Paulinho16]

No description provided.

[RobertLRead]

I will try to do this tomorrow.

On Sat, Nov 13, 2021 at 3:36 PM Paulinho16 ***@***.***> wrote: @Paulinho16 <https://github.com/Paulinho16> requested your review on: #24 <#24> Issues #16 <#16> and #19 <#19>. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#24 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABINEH2ZRXEI6PFXI3XCOQDUL3K6NANCNFSM5H6JFIMQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

-- Robert L. Read, PhD Twitter: @RobertLeeRead @pubinvention Public Invention: https://www.pubinv.org Join Our Mailing list: ***@***.*** YouTube: https://www.youtube.com/channel/UCJQg_dkDY3KTP1ybugYwReg Medium: ***@***.***

[RobertLRead]

Dear @Paulinho16 ,

I've reviewed this, but I'm going to check it out and test it; if it works i will merge it. However, there are some things that I would like done differently. You put the css directly on the objects in some cases, and I would rather ALL of it go into the .css file. In the .css file, you use the #id syntax to name multiple object, but I think we have a class for that (if we don't have a class, maybe we can add one.) I could be wrong about that, Web components are little clunky around classes, so possibly your approach is best, but it is fragile---if we add a new component we will have to remember to change it in the .css and that will take a while to track down.

[RobertLRead]

I just tested this, I'm going to merge it. The font sizes are nicer.

However, I don't believe we should specify these in "pixels", but that varies a lot with the nature of the device we're using (phones have low resolution than big monitors.) I believe the more professional way to do that is to use "ems" as the unit of the abstract names "large, x-large, huge" which are defined by css; but I will merge this and create a separate issue for that.
Also, I believe the text box where they are entering the number should be EVEN LARGER than the other text in the left hand modal. Also, the background should change to black in a clinical display, but I will put all of that in issues.

[RobertLRead]

@Paulinho16 I should have used this approve feature, but added comments directly instead; next time I will use this feature better, thank you.

[Paulinho16]

Sounds great. I’ll be working on the minor fixes in the mean time. Have a great day.

…

On Sun, Nov 14, 2021 at 10:39 AM Robert L. Read ***@***.***> wrote: Merged #24 <#24> into master. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#24 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ARTQAXUFJ2BRJ2ZTKVVYRALUL767VANCNFSM5H6JFIMQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.