-
Notifications
You must be signed in to change notification settings - Fork 10.5k
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
Share Functions under single button in MessageBox #4033
Conversation
@dytec Thanks for giving it a go - I'm not able to try it out for myself as it seems that Apple decided to stop producing Safari for Windows. :/ My housemate has a Macbook, so I'll be able to trial it once I get home today. |
@tgxn I´ve digged a little bit and it seems that this behavior is not related to your PR. The following screenshots are from a fresh pull of the develop branch without your PR: I´ve to dig deeper... |
BTW it seems that in Safari the Geolocation API is available ONLY when connected to WIFI... |
Hmm, I think it might be due to the css allowing too much space for the buttons (when they are hidden, which happens when certain API's aren't available). |
@dytec we had a bug with Safari, did you test the latest version from Thanks for the PR @tgxn, and we agree the we should have the option to group all attachments into a single button, but we still need to keep the emoji on the oposite side, so we don't lose the alignment with the text on top. |
@engelgabriel Yeah fresh download from develop a few minutes ago... |
@engelgabriel |
…e Attachments, added the option to group the attachment buttons, added i18n strings for new settings, updates existing attachment packages to reflect new settings section
Updated screenshots: Now I also tested out some different combination of enabled/disabled options, here's some examples. I would also like to as the Rocket.Chat team's opinion on Issue #3991 - Regarding location permission onload rather than onrequest. I think it would be best if (if enabled) the button should show up for everyone, and if the location API is unavailable on the user's device, we can show an error post-click. This resolves the "Why is this website asking for my location on load" and "How do we know if the user has the right location APIs enabled" by pushing the responsibility back onto the user to have an available location API, and we can also guide users on how to enable/allow location access. |
@tgxn you rock! Thanks for the contribution! The more attachment types we add the worse this has gotten. This is a fantastic addition! 👍 Will take a look at mentioned issue. |
@RocketChat/core
@rodrigok
Closes:
#4038 - Incorrect conditional when rendering the send button.
Took some time to think about how this would best work, and I think this comes close to a implementation that I'm happy with.
Based on PR #4006, I moved the buttons around and added an expanding section for the "Share" buttons.
Still requires browser testing, as I only tested in Chrome and Mobile Chrome/Rocket.Chat App (Android).
LTR
= Hidden
= Expanded
RTL
= Hidden
= Expanded