Closed wincowgerDEV closed 2 years ago
wincowgerDEV
commented
2 years ago @zsteinmetz I think that the translation PR is going to take a few months at least to implement so I am hoping that for now, we can just review and merge the UI updates that are within it which I have separated here.
wincowgerDEV
commented
2 years ago This contains all added components from #106 so that can be merged after this is or just deleted.
wincowgerDEV
commented
2 years ago Sorry for the confusion, I meant to delete the app.R file, it was just a concept I tested out. Use the ui.R and server.R files. Yeah I felt the same way when I tried to open this up again after a few months, crazy how fast we forget it. What kind of errors are you getting?
Cheers Win
On Wed, Jan 12, 2022, 6:42 AM Zacharias Steinmetz @.***> wrote:
@.**** commented on this pull request.
What was your reason for adding app.R and not keeping the UI and the Shiny server separated in two R files? Which one is supposed to work? I get errors for both. Sorry, I'm confused. Maybe it has been too long that I worked with the code 🙈
— Reply to this email directly, view it on GitHub https://github.com/wincowgerDEV/OpenSpecy/pull/110#pullrequestreview-850573646, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGMUJU6DWYQMFAENO7FZ773UVWHOHANCNFSM5LEATHCQ . 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.
You are receiving this because you authored the thread.Message ID: @.***>
wincowgerDEV
commented
2 years ago That's a relic, thanks for finding it!
On Thu, Jan 13, 2022, 3:18 AM Zacharias Steinmetz @.***> wrote:
@.**** commented on this pull request.
In inst/shiny/server.R https://github.com/wincowgerDEV/OpenSpecy/pull/110#discussion_r783864544 :
@@ -91,7 +111,8 @@ load_data <- function() { server <- shinyServer(function(input, output, session) {
For theming
bs_themer()
-
- observe_helpers()
What is the purpose of calling observe_helpers() here? It's part of the shinyhelpers package not defined above. I commented out in my local repo for now without missing any functionality. If it's not a relic but needed, we should define the dependency properly.
— Reply to this email directly, view it on GitHub https://github.com/wincowgerDEV/OpenSpecy/pull/110#pullrequestreview-851578880, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGMUJU73DPETNXQMIR2MBJ3UV2YJTANCNFSM5LEATHCQ . 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.
You are receiving this because you authored the thread.Message ID: @.***>
wincowgerDEV
commented
2 years ago YUP!
On Thu, Jan 13, 2022 at 5:58 AM Zacharias Steinmetz < @.***> wrote:
@.**** commented on this pull request.
In inst/shiny/server.R https://github.com/wincowgerDEV/OpenSpecy/pull/110#discussion_r783981635 :
Translate page ----
Also a relic, I suppose?
— Reply to this email directly, view it on GitHub https://github.com/wincowgerDEV/OpenSpecy/pull/110#pullrequestreview-851746485, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGMUJUYBBC4PFLQQ6CWO4JTUV3LBPANCNFSM5LEATHCQ . 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.
You are receiving this because you authored the thread.Message ID: @.***>
--
·.¸¸.·´¯·.´¯·.¸¸.·´¯ツ ------------------------------Win Cowger, PhD Pronouns: he/him Research Scientist Moore Institute for Plastic Pollution Research
Contact Info
515-298-3869 | @.*** | @Win_OpenData https://twitter.com/Win_OpenData
wincowgerDEV
commented
2 years ago I just made one more tiny UI fix to remove the UI panels for revolutionizing so that it is obvious what levels of support we have at first glance. I am good with your updates.
zsteinmetz
commented
2 years ago Two last things, sorryy:
Have you experienced something similar?I got the following error:
This Font Awesome icon ('gear') does not exist:
* if providing a custom `html_dependency` these `name` checks can
be deactivated with `verify_fa = FALSE`Exchanging 'gear' with 'cog' as recommended here made the error message go away but the UI still looks the same.
wincowgerDEV
commented
2 years ago Yeah I agree that the new scrolling is a bit of a poor point. I wanted something more like a right arrow click function to show the next item so that the whole first tweet and the whole first support would be visible but I couldn't get it to work. Any ideas? I'm not sure how removing a few of the tweets would improve the scrolling but I'm down to give it a try.
I hadn't experienced the gear error but the cog alternative sounds good to me.
On Fri, Jan 21, 2022, 3:38 AM Zacharias Steinmetz @.***> wrote:
Two last things, sorryy:
- I'm not so super pleased with the additional scrolling needed to view the support and Twitter panels. While I suggest to simply enlarge the support panels, we could randomly chose two or three tweets every time the page is loaded.
- The "gear" icons are not displayed on my local machine anymore: [image: image] https://user-images.githubusercontent.com/10266826/150520231-e0277de3-05d0-4991-8eb7-8de536c2eadc.png Have you experienced something similar?
I got the following error:
This Font Awesome icon ('gear') does not exist:* if providing a custom
html_dependencythesenamechecks can be deactivated withverify_fa = FALSEExchanging 'gear' with 'cog' as recommended here https://github.com/jlab/FlowSoFineApp/issues/1 made the error message go away but the UI still looks the same.
— Reply to this email directly, view it on GitHub https://github.com/wincowgerDEV/OpenSpecy/pull/110#issuecomment-1018429759, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGMUJU4HVCIPK6PGWXDLJFLUXFASBANCNFSM5LEATHCQ . 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.
You are receiving this because you authored the thread.Message ID: @.***>
zsteinmetz
commented
2 years ago 
What do you think about such a landing page?
ardcarvalho
commented
2 years ago
What do you think about such a landing page?
Definitely better than the current one!
zsteinmetz
commented
2 years ago Alright then, I'll push it. I also changed the appearance of tweets a bit. It's only two at a time now but they randomly change when the page is reloaded.
zsteinmetz
commented
2 years ago I hadn't experienced the gear error but the cog alternative sounds good to me.
Even when using "cog" instead of "gear", the toggles look like this on my machine:
Are they still supposed to look like this?
wincowgerDEV
commented
2 years ago Well done! Love it :)
On Wed, Feb 9, 2022 at 8:15 AM Aline Carvalho @.***> wrote:
[image: image] https://user-images.githubusercontent.com/10266826/153236706-c33cebed-3435-436a-a55d-a4978c6ec0c0.png
What do you think about such a landing page?
Definitely better than the current one!
— Reply to this email directly, view it on GitHub https://github.com/wincowgerDEV/OpenSpecy/pull/110#issuecomment-1033935226, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGMUJUZGI7BFTEBXEODXO2LU2KHJRANCNFSM5LEATHCQ . 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.
You are receiving this because you authored the thread.Message ID: @.***>
--
·.¸¸.·´¯·.´¯·.¸¸.·´¯ツ ------------------------------Win Cowger, PhD Pronouns: he/him Research Scientist Moore Institute for Plastic Pollution Research
Contact Info
515-298-3869 | @.*** | @Win_OpenData https://twitter.com/Win_OpenData
wincowgerDEV
commented
2 years ago Yeah the top one is the one I intended to make it, that way people could see what was active in the bottom window, open to alternative options though.
On Wed, Feb 9, 2022 at 8:26 AM Zacharias Steinmetz @.***> wrote:
I hadn't experienced the gear error but the cog alternative sounds good to me.
Even when using "cog" instead of "gear", the toggles look like this on my machine:
[image: image] https://user-images.githubusercontent.com/10266826/153244077-b880ae66-4c05-40b9-9eaf-0d7707235f4c.png
Are they still supposed to look like this?
[image: image] https://user-images.githubusercontent.com/10266826/153244266-8a8e9bfb-9f08-4a83-a3c1-53afca871a2c.png
— Reply to this email directly, view it on GitHub https://github.com/wincowgerDEV/OpenSpecy/pull/110#issuecomment-1033947795, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGMUJU3KIQ4CNPXQJ3BPLYLU2KIUVANCNFSM5LEATHCQ . 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.
You are receiving this because you authored the thread.Message ID: @.***>
--
·.¸¸.·´¯·.´¯·.¸¸.·´¯ツ ------------------------------Win Cowger, PhD Pronouns: he/him Research Scientist Moore Institute for Plastic Pollution Research
Contact Info
515-298-3869 | @.*** | @Win_OpenData https://twitter.com/Win_OpenData
zsteinmetz
commented
2 years ago And "adv" is short for "advanced options"? I'll try to give it a second thought. Maybe we can find something more intuitive.
zsteinmetz
commented
2 years ago I'd opt for something like this:
zsteinmetz
commented
2 years ago @wincowgerDEV, should I squash and merge this commit?
wincowgerDEV
commented
2 years ago Sorry just a few last things haha :)
I thought that the red color with the eye seemed a little scary and didn't want it to seem like an error to untrained user. How about this? I am going to add this to the final commit plus a couple of other popover bugs I just found where the popovers don't show if we print any of the text on a new line.
wincowgerDEV
commented
2 years ago BTW @zsteinmetz and @ardcarvalho I finally got Open Specy up on a more professional looking EC2 server, been spending the whole week on this. https://openanalysis.org/openspecy/
By having the tool on the server, we have more control over how we scale while more users jump on. Let me know what you think .
zsteinmetz
commented
2 years ago I finally got Open Specy up on a more professional looking EC2 server
Awesome :heart_eyes: it's lightning fast!! I like the gray color. I'll add it to main.
wincowgerDEV
commented
2 years ago Thanks :)
On Sat, Feb 12, 2022 at 5:38 AM Zacharias Steinmetz < @.***> wrote:
Merged #110 https://github.com/wincowgerDEV/OpenSpecy/pull/110 into main.
— Reply to this email directly, view it on GitHub https://github.com/wincowgerDEV/OpenSpecy/pull/110#event-6056980569, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGMUJU5S74AIXJTVLWNRUG3U2ZPEVANCNFSM5LEATHCQ . 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.
You are receiving this because you were mentioned.Message ID: @.***>
--
·.¸¸.·´¯·.´¯·.¸¸.·´¯ツ ------------------------------Win Cowger, PhD Pronouns: he/him Research Scientist Moore Institute for Plastic Pollution Research
Contact Info
515-298-3869 | @.*** | @Win_OpenData https://twitter.com/Win_OpenData
ardcarvalho
commented
2 years ago BTW @zsteinmetz and @ardcarvalho I finally got Open Specy up on a more professional looking EC2 server, been spending the whole week on this. https://openanalysis.org/openspecy/
This is amazing! And indeed, super fast.
Just the formatting that is lost in the mobile version (for both) and looking gets worse. Hard to configure this? (Imagining you've struggled with...)
wincowgerDEV
commented
2 years ago Hey Aline,
Yeah, I have noticed that. I guess I figured that few people would want to analyze spectra from their cell phone so I haven't gotten around to formatting it in a way that would be compatible with the cell phone but it is a good thing to think about if we end up getting some people who do want to use their phone. I think the next version of Open Specy's UI will be simpler and therefore easier for us to program in the conversion so it might be a good idea to bring up with the UI team now.
Warm Regards Win
On Sun, Feb 13, 2022 at 10:56 PM Aline Carvalho @.***> wrote:
BTW @zsteinmetz https://github.com/zsteinmetz and @ardcarvalho https://github.com/ardcarvalho I finally got Open Specy up on a more professional looking EC2 server, been spending the whole week on this. https://openanalysis.org/openspecy/
This is amazing! And indeed, super fast.
Just the formatting that is lost in the mobile version (for both) and looking gets worse. Hard to configure this? (Imagining you've struggled with...)
— Reply to this email directly, view it on GitHub https://github.com/wincowgerDEV/OpenSpecy/pull/110#issuecomment-1038717056, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGMUJU67P3ATJC7K7VX6H5LU3CRSBANCNFSM5LEATHCQ . 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.
You are receiving this because you were mentioned.Message ID: @.***>
--
·.¸¸.·´¯·.´¯·.¸¸.·´¯ツ ------------------------------Win Cowger, PhD Pronouns: he/him Research Scientist Moore Institute for Plastic Pollution Research
Contact Info
515-298-3869 | @.*** | @Win_OpenData https://twitter.com/Win_OpenData
ardcarvalho
commented
2 years ago Yeah, I tought about that to, that few people would want to analyze spectra from their mobiles. On the other hand it's still a 'cover' to the project and, hum, appearance somehow matters.
Great that we've UI team now!
wincowgerDEV
commented
2 years ago True that! I think the front about page for Open Specy should look good on all platforms. Definitely adding this to the UI team focus.
Warm Regards, Win
On Thu, Feb 17, 2022 at 7:19 AM Aline Carvalho @.***> wrote:
Yeah, I tought about that to, that few people would want to analyze spectra from their mobiles. On the other hand it's still a 'cover' to the project and, hum, appearance somehow matters.
Great that we've UI team now!
— Reply to this email directly, view it on GitHub https://github.com/wincowgerDEV/OpenSpecy/pull/110#issuecomment-1043062048, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGMUJU5MKXWIF6JDMXCC64LU3UGZVANCNFSM5LEATHCQ . 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.
You are receiving this because you were mentioned.Message ID: @.***>
--
·.¸¸.·´¯·.´¯·.¸¸.·´¯ツ ------------------------------Win Cowger, PhD Pronouns: he/him Research Scientist Moore Institute for Plastic Pollution Research
Contact Info
515-298-3869 | @.*** | @Win_OpenData https://twitter.com/Win_OpenData
These are just the UI updates that are implemented in the language translation PR. I think these UI updates are worthwhile, better credit for contributors and a better processing panel that doesn't cover up the window where the plots are. Also some nicer text in places and added tweets.