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

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

Closed
ivan-aksamentov opened this issue Mar 23, 2020 · 12 comments · Fixed by #139 or #144
Closed

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

ivan-aksamentov opened this issue Mar 23, 2020 · 12 comments · Fixed by #139 or #144
Labels
help wanted Extra attention is needed s:ui Scope: related to user experience, user interface, usability, accessibility t:bug Type: bug, error

Comments

@ivan-aksamentov
Copy link
Member

🐛 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

Related:
#20

@ivan-aksamentov ivan-aksamentov added t:bug Type: bug, error help wanted Extra attention is needed s:ui Scope: related to user experience, user interface, usability, accessibility labels Mar 23, 2020
@gj262
Copy link
Collaborator

gj262 commented Mar 23, 2020

Sounds like: reactstrap/reactstrap#1428

@ivan-aksamentov
Copy link
Member Author

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

@rebmullin
Copy link
Contributor

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

@gj262
Copy link
Collaborator

gj262 commented Mar 23, 2020

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

@rebmullin
Copy link
Contributor

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

@gj262
Copy link
Collaborator

gj262 commented Mar 23, 2020

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

@ivan-aksamentov
Copy link
Member Author

ivan-aksamentov commented Mar 23, 2020

@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
Copy link
Collaborator

gj262 commented Mar 23, 2020

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
Copy link
Collaborator

gj262 commented Mar 23, 2020

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
Copy link
Member Author

ivan-aksamentov commented Mar 23, 2020

@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 #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
Copy link
Collaborator

gj262 commented Mar 23, 2020

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
Copy link
Member Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed s:ui Scope: related to user experience, user interface, usability, accessibility t:bug Type: bug, error
Projects
None yet
3 participants