[MM-52207] Grammarly plugin not detected in Mattermost desktop #24081
Conversation
mm-cloud-bot
added
the
release-note-none
|
Hello @hasancankucuk, Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here. |
| <label id="advancedTextEditorCellAriaLabel" style={{ display: "none", visibility: "hidden", height: 0 }}>{Utils.localizeMessage( | ||
| 'channelView.login.successfull', | ||
| 'Login Successfull', | ||
| ) + ' ' + ariaLabelMessageInput}</label> |
There was a problem hiding this comment.
I am not sure if this is exactly how this issue is supposed to be solved, honestly, I don't have the solution either. Please note that the issue is currently doesn't appear to be in Mattermost run in either Chrome, Firefox, but in Desktop app only.
There was a problem hiding this comment.
@M-ZubairAhmed After testing the changes I made to Mattermost on MacOS, it appears that the problem has been successfully resolved. I have attached screenshots for reference. The reason behind this solution is that when aria-label is added, Grammarly treats the textarea as a string, making it unable to detect. To address this issue, I included a label to prevent the textarea from being treated as a string, ensuring both accessibility is maintained and Grammarly detects it correctly. I conducted comprehensive testing using VoiceOver and Grammarly version 1.30.2.0 to validate the improvements
**
| 
| 
|**
There was a problem hiding this comment.
@hasancankucuk i see but i am curious why it only happens on desktop app and not not webapp. Because if Grammarly is does able to identify in webapp, ideally it should do the same for electron as well. Thats being said I would like to explore if this issue is just electron specific and if there is electron specific apis we would like to see to make Grammarly work over there.
|
@hasancankucuk i have updated the original issue can you please check |
|
Creating a new SpinWick test server using Mattermost Cloud. |
|
Enterprise Edition Image not available in the 30 minutes timeframe, checking the Team Edition Image and if available will use that. |
|
Test server creation failed. See the logs for more information. |
|
@M-ZubairAhmed While looking for another solution to the problem, I found useful information in React's official documentation. The accessibility information provided by React matches the solution I made. Here is the link for your reference if you want to have a look.
Documentation Link: https://react.dev/reference/react-dom/components/textarea |
M-ZubairAhmed
requested review from
hmhealey
and removed request for
M-ZubairAhmed
hmhealey
added
2: Dev Review
|
E2E tests not automatically triggered, because the PR is not in a mergeable state. Please update the branch with the base branch and resolve outstanding conflicts. |
There was a problem hiding this comment.
Thanks for the PR! I haven't been able to test the PR yet since I don't have Grammarly installed myself, but I do have some feedback first before I can get someone else to test this.
In addition to my comments on the code, could you also ensure the unit tests for the web app run by running npm test -w channels from the webapp folder? Based on the CI results, those are currently failing because the snapshots of the AutosizeTextarea component have changed
webapp/channels/src/components/advanced_text_editor/advanced_text_editor.tsx
Outdated
Show resolved
Hide resolved
webapp/channels/src/components/advanced_text_editor/advanced_text_editor.tsx
Outdated
Show resolved
Hide resolved
|
This PR has been automatically labelled "stale" because it hasn't had recent activity. |
devinbinnie
added
Setup Cloud Test Server
|
Creating a new SpinWick test server using Mattermost Cloud. |
|
Enterprise Edition Image not available in the 30 minutes timeframe, checking the Team Edition Image and if available will use that. |
|
/update-branch |
|
Test server creation failed. See the logs for more information. |
|
/e2e-test |
|
Successfully triggered E2E testing! |
| <label id="advancedTextEditorCellAriaLabel" style={{ display: "none", visibility: "hidden", height: 0 }}>{Utils.localizeMessage( | ||
| 'channelView.login.successfull', | ||
| 'Login Successfull', | ||
| ) + ' ' + ariaLabelMessageInput}</label> |
There was a problem hiding this comment.
@hasancankucuk i see but i am curious why it only happens on desktop app and not not webapp. Because if Grammarly is does able to identify in webapp, ideally it should do the same for electron as well. Thats being said I would like to explore if this issue is just electron specific and if there is electron specific apis we would like to see to make Grammarly work over there.
| {Utils.localizeMessage( | ||
| 'channelView.login.successfull', | ||
| 'Login Successfull', | ||
| ) + ' ' + ariaLabelMessageInput}</label> |
There was a problem hiding this comment.
please use react intl methods here
There was a problem hiding this comment.
I don't think it's a problem specific to the desktop application. When I tested it on the master branch in Safari, it still couldn't detect Grammarly Mattermost as in the screenshots.
|
New commit detected. SpinWick will upgrade if the updated docker image is available. |
|
Test server creation failed. See the logs for more information. |
yasserfaraazkhan
removed
the
Setup Cloud Test Server
The aria-label attribute has been modified, and it is impacting the text-area property. Grammarly couldn't detect the issue since the text-area was affected.
hasancankucuk
force-pushed
the
MM-52207-grammarly-plugin-detection-fix
branch
from
7421dba
to
35e6b72
Compare
|
@devinbinnie Grammarly is now detecting Mattermost Desktop v5.5.1 both local build and the dmg downloaded from https://docs.mattermost.com/collaborate/install-desktop-app.html . But on the App Store version, it's still not working. On Grammarly's support page, there's info about this situation. I've also checked Mattermost Desktop's electron version and it currently uses version 26.2.1. I think the issue and pull request can be closed. I'm also attaching a screenshot and a link to Grammarly's support page. |
Y
Yeah, this is unfortunately the case for a lot of things. In order to be in the App Store, macOS requires that an app be placed within its App Sandbox that restricts access to a lot of functionality, unless specifically granted. This probably means that Grammarly won't work with our app in the App Store, so this change is the best we can do. |
|
@M-ZubairAhmed Are we okay to approve this? |
|
Foloww
Following the above discussion i think this is the maximum we can do from outside. Let me rope in @yasserfaraazkhan to test if this fix works for the following
Please check with and without the above fix. Also please use the browser Grammarly plugin and not the app. |
M-ZubairAhmed
changed the title
I just downloaded version 5.6.0-rc.1 from Github for Mac M1, and still Grammarly it's not detected |
|
/update-branch |
|
@hasancankucuk please fix the CI for the preview url to get generated |
|
This PR has been automatically labelled "stale" because it hasn't had recent activity. |
yasserfaraazkhan
removed
the
3: QA Review
Summary
The aria-label attribute has been modified, and it is impacting the text-area property. Grammarly couldn't detect the issue since the text-area was affected.
Ticket Link
Fixes: #22970
Jira: https://mattermost.atlassian.net/browse/MM-52207
Screenshots
|
| 
|
Release Note