fix addressing image block overlay layout shift problems #85
Conversation
|
|
||
| useEffect(() => { | ||
| if (!isOpened) { | ||
| document.body.style.overflow = "visible"; |
There was a problem hiding this comment.
p1: It cause layout shift (you know scroll bar has its own width)
There was a problem hiding this comment.
p1: you'd better research other library
but don't use it. just copy code
| transform: `scale(${scale})`, | ||
| transformOrigin: `${scaleOriginX * 100}% ${scaleOriginY * 100}%`, | ||
| cursor: getCursorStyle(scale), | ||
| cursor: isCursorVisible ? getCursorStyle(scale) : "none", |
There was a problem hiding this comment.
p1: cursor does not disappear on overlay (not image). you should fix it
| }, [isOpened, currentImageIndex, setScale, setDisplayScale]); | ||
|
|
||
| useEffect(() => { | ||
| const notion = document.querySelector(".notion") as HTMLElement; |
There was a problem hiding this comment.
p1: scroll on .notion does not work
There was a problem hiding this comment.
Is that still the case even when the image viewer is not open?
There was a problem hiding this comment.
umm.. scrolling element is not .notion
it depends on user's layout
|
|
||
| if (isOpened) { | ||
| document.body.style.overflow = "hidden"; | ||
| notion.style.marginRight = "15px"; |
There was a problem hiding this comment.
p1: margin '15px' is dangerous considering cross brouwsing
you should research 3rd libraries code
packages/core/src/lib/index.css
Outdated
| margin-right: 15px; | ||
| } | ||
|
|
||
| button { |
There was a problem hiding this comment.
p1: don't define global button style
|
|
||
| export const useCursorVisibility = (isScaleFocus: boolean) => { | ||
| export const useCursorVisibility = () => { | ||
| const [isCursorVisible, setIsisCursorVisible] = useState(true); |
There was a problem hiding this comment.
p1: setIsis -> typo error
|
p3: you should give more detail info on pr title |
myjeong19
changed the title
|
I used react-remove-scroll-bar and react-style-singleton as references to resolve the layout shift issues. |
|
|
||
| if (isOpened) { | ||
| document.body.setAttribute("data-scroll-locked", "true"); | ||
| const gap = getGapWidth(); |
There was a problem hiding this comment.
p2: you should capsulize this hook.
I suggest use-prevent-scroll
There was a problem hiding this comment.
I checked this review.
2 participants
Description of Changes
Here is reference: #25
support 'custom your created image' block
Review point
To reproduce
Screenshot
Review Guide
Reviews are conducted based on priority levels, such as p0, p1, p2, p3, p4, and p5.
p0 ~ p2: If the author decides not to reflect a review for p0 to p2, it signals that a proper discussion with the reviewer is
necessary. It is expected that the review will be resolved either through incorporating the feedback or through further discussion.
p3: indicates that the reviewer has identified a significant issue, but either lacks a clear solution or the comment lacks sufficient context. Further explanation or additional discussion on the reviewer's concerns is needed.
p4, p5: p4 and p5 suggest low priority, and if the author does not deem them important, these comments can be disregarded.