[Web] Fix scrolling in Node list in web terminal#26622
Conversation
ravicious
left a comment
ravicious
left a comment
There was a problem hiding this comment.
It fixes the problem but I think we should pursue the solution with removing overflow: hidden if possible. Doing so will make it harder to reintroduce this problem in the future.
| display: flex; | ||
| flex: 1; | ||
| max-width: 1024px; | ||
| height: fit-content; |
There was a problem hiding this comment.
Do you remember why overflow: hidden was added to StyledTableWrapper? If it's possible to get rid of it then I think long-term it might be a better fix than adding height: fit-content here.
There was a problem hiding this comment.
The reasoning was for all our different tables (paginated, searchable, simple, etc.) to have rounded corners. Wrapping all tables with it and having it round the corners with a borderRadius made it so we didn't have to always manually add a border radius for every table individually.
| participantModes: ParticipantMode[]; | ||
| showCTA: boolean; | ||
| }) => { | ||
| const [anchorEl, setAnchorEl] = useState<HTMLElement>(null); |
There was a problem hiding this comment.
A feedback for the future as this pattern wasn't introduced by this PR specifically, but I don't think it's a good practice to store HTML elements in the state – you might end up storing an element in the state that's no longer in DOM. Was this done as a workaround for some other problem?
Looking at the previous version of JoinMenu, I think a more idiomatic solution would be to create a new ref with useRef, pass it as the ref prop to <ButtonBorder>. Then to control whether <Menu> is supposed to be open or not, we could have const [isOpen, setIsOpen] = useState(false) and on the button click we could flip it to true.
There was a problem hiding this comment.
MenuLogin does it this way already. It looks like the prop has to be named setRef for the button components, I'm not exactly sure why it's done like this.
teleport/web/packages/design/src/Button/Button.jsx
Lines 23 to 25 in 2bb3860
There was a problem hiding this comment.
Agreed, I'll keep track of this and we can come back and refactor all our popover menu components at some point.
public-teleport-github-review-bot
Bot
removed the request for review
from kimlisa
3 participants
Purpose
This PR resolves #26604
Fixes the scrolling on the Nodes list in the web terminal.
Also, for active sessions, makes it so the participant mode menu closes after joining a session rather than staying open.