neherlab / covid19_scenarios

Models of COVID-19 outbreak trajectories and hospital demand

https://covid19-scenarios.org

MIT License

1.36k stars 352 forks source link

In Safari help text does not appear on clicking the help buttons #134

Closed ivan-aksamentov closed 4 years ago

ivan-aksamentov commented 4 years ago

🐛 Bug Report

In Safari help text does not appear on clicking the help buttons

This was reported by @Johan-A-M in chat: https://spectrum.chat/covid19-scenarios/general/questions-discussions~8d49f461-a890-4beb-84f7-2d6ed0ae503a?m=MTU4NDkwODk0NTM5MQ==

🤔 Expected Behavior

Clicking on help button should bring up the popup with explanation text

😯 Current Behavior

Apparently, nothing happens on click when browsing in Safari on Mac

💁 Possible Solution

Unknown

🔦 Context

Not specified

💻 Code Sample

Not specified

🌍 Your Environment

Software	Version(s)
Browser	Safari, unspecified version
Operating System	Mac, , unspecified version
NPM/Node/Yarn

gj262 commented 4 years ago

Sounds like: https://github.com/reactstrap/reactstrap/issues/1428

ivan-aksamentov commented 4 years ago

@gj262 Good catch! Do you have a possibility to test on Safari?

rebmullin commented 4 years ago

hi @ivan-aksamentov, I just created a patch for this. Please let me know if anything is needed. thanks :)

gj262 commented 4 years ago

@ivan-aksamentov I do. It looks like many people are on this bug! I'll see how those PRs go.

rebmullin commented 4 years ago

I didn't realize so many people were working on this one! i can close mine if needed too:)

gj262 commented 4 years ago

Hi @rebmullin I tried out your PR but it does not work for my safari. I'll give #138 a go.

ivan-aksamentov commented 4 years ago

@rebmullin @gj262 That was a fast response to user's bug report :)

In fact I reopen to accept confirmation that it indeed works on Safari

gj262 commented 4 years ago

I took a look at: http://reactstrap.github.io/components/popovers/#Popovers-Trigger in safari. The 'legacy' and 'click' methods work and perhaps we can simply use one of those for the Popover. I'll give that a try.

gj262 commented 4 years ago

Ok. The following works with chrome and safari.

Use an UncontrolledPopover rather than a Popover.
Use trigger="legacy"
Keep the onClick prop on Button to avoid triggering dropdown toggle for labels that have them. i.e. Results.

In Chrome the help click behaves as present. In Safari the popover appears but the same help icon has to be clicked again to hide it. In Chrome you can click anywhere to hide. So sub optimal for Safari but better than not being able to view help.

@ivan-aksamentov let me know if this works. I'll throw up a PR if I can get the test to pass.

ivan-aksamentov commented 4 years ago

@gj262 We wanted these popovers to contain links and other rich content later on, with more thorough explanations. People might select and copy-paste text for example. This would help with issues like this https://github.com/neherlab/covid19_scenarios/issues/137

I think the UncontrolledPopover disappears when you try to click on it or interact in any other way. Is there a way to polyfill maybe?

How about rip this thing off for good and start over? Either using another library or write a component from scratch. I am sure someone somewhere should have this problem solved...

In the meanwhile, I will accept the PR though. We basically have all the Safari users in the dark now. And these are probably the most of our medical users.

gj262 commented 4 years ago

Hi @ivan-aksamentov,

UncontrolledPopover with trigger="legacy" does allow for text selection.

Re: start from scratch. I'd avoid it. What you have works well enough. Other libraries will have their own issues. A homegrown popover will have its own issues. So until you are ready to completely redesign and jettison Bootstrap, I'd live with it. (my quick take after only helping out for 2 days). :)

Side note: Do you have a plan to add react-storybook or similar? That is great for visualizing components in different states e.g. in this case a help popover with rich content. I tend to rely on it after tests to ensure the components give me what I need.

Cheers, -Gavin

ivan-aksamentov commented 4 years ago

@gj262 Okay then, let's see how it goes.

Storybook is great, but I am not quite sure who will be maintaining it. Not that we have a dedicated designer. It will probably be permanently broken. If you feel like this is something useful, and if you can come by time to time and fix things, then go ahead and add it, project will only become better I think.