-
Notifications
You must be signed in to change notification settings - Fork 51
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
fix: Mentions doesnt work on chrome and on firefox #280
Conversation
Warning Rate limit exceeded@ilyassBZ has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 25 minutes and 16 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Walkthrough此次更改涉及对 Changes
Assessment against linked issues
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Can a test case be added to cover this scene? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #280 +/- ##
=======================================
Coverage 98.37% 98.38%
=======================================
Files 7 7
Lines 246 247 +1
Branches 58 58
=======================================
+ Hits 242 243 +1
Misses 4 4 ☔ View full report in Codecov by Sentry. |
@yoyo837 done |
Conflicting. |
fixed.. plus keyCode.ALT doesnt work in azert... no need for it |
if ( | ||
key === nextMeasurePrefix || | ||
key === 'Shift' || | ||
which === KeyCode.ALT || | ||
key === 'AltGraph' || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Jarryxin Is this correct? Should #281 already resolve the problem?
可能确实不一样,我是用港版mac改成法语,测试alt是 18,在我修改的版本发邮件给提问者,他说是正确的。是否应该两种alt都兼容?
Maybe it's different. I changed from using the Hong Kong version of Mac to French, and the alt test was 18. In the revised version I provided to the questioner, he said it was correct. Should both alt versions be compatible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ant-design/ant-design#50795 (comment) in this comment , the alt keycode is 18
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Im azerty user we have 2 Alt keys the one u talked about and the other one it AltGr that it been user to do @ cus in azerty keyboard just normal Alt + à dosnt do anything AltGr does
I guess in mac in diffrent or something
In azert keyboard we have left alt whiche it all keyboard and the right AltGr with it only in azerty keyboard
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
U can merge both do if u wanna satay safe in case ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i guess we should merge both of them do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cus in ur coode u looking for the key number which it 18 OR 225 depending on keyboard but key name gonna stay the same which it AltGraph it dosnt change ... that's make using key=== "AltGraph" moore affection than using which key number but im fine with merging both of them if thats gonna suit u @yoyo837 @Jarryxin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
KeyCode.ALT
or AltGraph
? Which one is correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i guess we should merge both of them do
Should we keep KeyCode.ALT
?
@yoyo837 any update for the merge ? |
Personally, I think it's better to merge both just to be safe. |
sure should i add it for u can merge both of them or u gonna add it from ur side ? |
Co-authored-by: afc163 <[email protected]>
Co-authored-by: afc163 <[email protected]>
Co-authored-by: afc163 <[email protected]>
Co-authored-by: afc163 <[email protected]>
fixing mention dosnt work on azer keyboard:
issues:
fix ant-design/ant-design#50795,
Summary by CodeRabbit
Summary by CodeRabbit