-
-
Notifications
You must be signed in to change notification settings - Fork 282
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
Bug Fix - Tab Crashes when page limit exceeds a certain amout (e.g. 10000) on explore community tab. #2109
base: dev
Are you sure you want to change the base?
Conversation
…in the Explore community tab
All contributors have signed the CLA ✍️ ✅ |
I have read the CLA Document and I hereby sign the CLA |
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.
Hey! I think adding a dialog for it is not really a good idea and instead we can set the input max
to something around 1152
for blocking user from entering high range where application might break.
onChange={(e: React.ChangeEvent<HTMLInputElement>) => { | ||
if (parseInt(e.target.value) > 10000) { | ||
e.target.value = e.target.value.slice(0, 4); | ||
} | ||
}} |
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.
please remove the onChange
as it is not required after adding max
and also will break the input from changing value.
<Chip variant="SurfaceVariant" onClick={() => setLimit('10000')} radii="Pill"> | ||
<Text size="T200">Max</Text> | ||
</Chip> |
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.
It won't be a good idea to show this option considering that the performance will be poor.
</Box> | ||
</Box> | ||
<Box as="form" onSubmit={handleLimitSubmit} direction="Column" gap="300"> | ||
<Box direction="Column" gap="100"> | ||
<Text size="L400">Custom Limit</Text> | ||
<Input | ||
placeholder="Max limit- 10000" |
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.
that placeholder is misleading and not required as input already has a label
@@ -45,6 +45,7 @@ import { getMxIdServer } from '../../../utils/matrix'; | |||
import { stopPropagation } from '../../../utils/keyboard'; | |||
import { ScreenSize, useScreenSizeContext } from '../../../hooks/useScreenSize'; | |||
import { BackRouteHandler } from '../../../components/BackRouteHandler'; | |||
// import { LimitWarning } from './LimitWarning'; |
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.
please remove the commented code
@@ -260,6 +261,7 @@ function LimitButton({ limit, onLimitChange }: LimitButtonProps) { | |||
if (!limitInput) return; | |||
const newLimit = limitInput.value.trim(); | |||
if (!newLimit) return; | |||
// my work here |
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.
here as well
Description
Fixes #2072
Before Changes:
https://github.com/user-attachments/assets/cc3674ad-18d6-40a2-9225-19569765daea
After Changes:
https://github.com/user-attachments/assets/1761a3b7-eb62-47a5-a766-e06dc2945887
Type of change
Checklist: