-
-
Notifications
You must be signed in to change notification settings - Fork 611
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
Support Box
overflow and scrolling
#432
Comments
Issue Status: 1. Open 2. Started 3. Submitted 4. Done This issue now has a funding of 1.0 ETH (2414.48 USD @ $2414.48/ETH) attached to it.
|
Based on what you're saying sounds like you might need to:
This is assuming you don't know the inner box contents from the beginning, since it could be rendered via flexbox rules. |
Issue Status: 1. Open 2. Started 3. Submitted 4. Done Work has been started. These users each claimed they can complete the work by 265 years, 7 months from now. 1) chrisirhc has applied to start work (Funders only: approve worker | reject worker). I think I have a solution that involves:
I would love to work on this issue, I have experience in reacting and I have already done something similar in a lib of angular components, it will be a very interesting challenge to conclude. I would love to work on this! Would solve the two issues, where a component may exceed the bounds of its container, in a timeframe of 5-7 days max. ขอความจริง และความชัดเจนในการทำงาน ที่ถูกต้อง ไม่อ้อมค้อม ทั้งภารกิจ วิธีการ และกำหนดเวลา ขอบคุณครับ Learn more on the Gitcoin Issue Details page. |
Here's a solution based on #393 : https://gist.github.com/chrisirhc/bf3f973e5d4db2cbc96d95d1f023f0ad |
@chrisirhc I don't see where your solution measures the full size of the inner contents... it seems like this only scrolls a known amount. I really want to be able to implement an up/down pager (think import React from "react";
import { Box, Text } from "ink";
import { loremIpsum } from "lorem-ipsum";
export const Paragraphs = ({ count = 10 }) => {
const raw = loremIpsum({
units: "paragraphs",
count
});
const text = raw.split("\n").join("\n\n");
return (
<Box>
<Text>{text}</Text>
</Box>
)
}
export const Main = () => {
return (
<Box width={80} height={25}>
<AmazeBoxRenderer>
<Box width={80}>
<Paragraphs count={10} />
</Box>
</AmazeBoxRenderer>
</Box>
);
} When I run this, I see To get to your other points,
I'm not sure this works, since Ink's renderer will respect the outer wrapper's bounds rather than letting some inner content render with whatever natural dimensions it has.
Definitely want to maintain this assumption. I'll write another comment to follow-up with some acceptance criteria, since I've thought of it. Regardless, thanks for taking an initial look @chrisirhc! Tricky problem, right? |
Acceptance criteria
This is about the extent of what I can think of as necessary, but of course I understand that there may be ambiguity or technical limitations when it comes to the details. Please don't hesitate to communicate any comments, questions, or concerns. Thank you to everyone who has thus far applied to work on this bounty! I will continue reviewing each of your applications and will respond to each individually in the next 48 hours. ❤️ |
@developerfred Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!
Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days |
Hi @gitcoinbot my code will be here https://replit.com/@codingsh/poc-ink-box-upgrade |
Issue Status: 1. Open 2. Started 3. Submitted 4. Done Work for 1.0 ETH (2311.76 USD @ $2425.56/ETH) has been submitted by:
|
@gnidan there is a way for me to get the height of the elements with this hook, I will write more test and improve the poc. |
Awesome @developerfred, glad to hear about progress! In the meantime, I grabbed the relevant stuff from my use case and made a repo for it at gnidan/ink-scroller. There's something strange going on with left/right scrolling, but I'm less focused on that, since I just haven't looked into it. Point of that repo: I want to be able to stop scrolling at the end of the text :) |
@developerfred any luck? Can I offer (free) help? |
@navxio sure! |
I think it would be interesting to have that as a separate npm package, but I'm not sure I'd want to have that in Ink itself. If you have a complicated CLI that requires scrolling areas, you might be better of with ncurses-based alternative like blessed. |
Box
overflow and scrolling
The issue with blessed and similar variants is we don't get the declarative dom-like environment that we get here... as it stands and (please someone correct me if I'm wrong), Ink is the only project that makes building a CLI or TUI more or less like building for the web... And I believe that's hugely important because we've come to realize the terminal offers one of the most productive, distraction free environments in existence... a turn-key solution to scrolling is absolutely fundamental in my opinion. Thanks for listening |
Glad to see some activity on this issue. I'll review the acceptance criteria and state of current efforts. I still want this, and I'm still willing to spend the listed bounty! Thanks for your insight @vadimdemedes and @zach-is-my-name 🙏 |
@vadimdemedes I remember from my exploration into this issue that we can't get a separate NPM package to work without a change to Ink's rendering methods... since Ink is responsible for doing all the layout math, it needs to keep track of the "full size" dimensions as well as the computed dimensions on-screen (what I call "natural size" vs. "fixed size" above), but the problem is that there's no way (right now) to create a react hook to report the natural size dimensions. It looks like this information gets lost somewhere deep inside the renderer (or worse, inside the Yoga/flexbox computations). This stuff is all very confusing to me, however, since Yoga doesn't (didn't?) have good API documentation. Maybe I'm wrong.
@developerfred is this still current? It seems rather barebones, and it doesn't currently even run at all (looks like you have a syntax error). Anyway, I notice there is some movement on #412 and also I see #472 is new since I've spent effort investigating the state of this issue. @vadimdemedes could you please comment on what a path forward might look like? I'm fine if it is actually possible to achieve this "natural size" calculation in a separate NPM package, but I just don't see how that's possible. If further coordination across existing PRs is necessary, I could potentially double the bounty and split it up across contributors/maintainers, since I really want this behavior, but mind you I'm only working with my personal funds here. But if this is a multi-person effort, I do want to recognize that and reward everyone's efforts. This behavior is the only thing stopping me from using Ink for a number of projects of varying sizes, and I can't move forward on those projects in a way that satisfies me (because, well, who wants to use blessed when ink exists ;) |
Love it. Thank you @zach-is-my-name. I'll watch that issue to see how this develops. |
Now that Here's an example const reducer = (state, action) => {
switch (action.type) {
case 'SET_INNER_HEIGHT':
return {
...state,
innerHeight: action.innerHeight
};
case 'SCROLL_DOWN':
return {
...state,
scrollTop: Math.min(
state.innerHeight - state.height,
state.scrollTop + 1
)
};
case 'SCROLL_UP':
return {
...state,
scrollTop: Math.max(0, state.scrollTop - 1)
};
default:
return state;
}
};
function ScrollArea({height, children}) {
const [state, dispatch] = React.useReducer(reducer, {
height,
scrollTop: 0
});
const innerRef = React.useRef();
React.useEffect(() => {
const dimensions = measureElement(innerRef.current);
dispatch({
type: 'SET_INNER_HEIGHT',
innerHeight: dimensions.height
});
}, []);
useInput((_input, key) => {
if (key.downArrow) {
dispatch({
type: 'SCROLL_DOWN'
});
}
if (key.upArrow) {
dispatch({
type: 'SCROLL_UP'
});
}
});
return (
<Box height={height} flexDirection="column" overflow="hidden">
<Box
ref={innerRef}
flexShrink={0}
flexDirection="column"
marginTop={-state.scrollTop}
>
{children}
</Box>
</Box>
);
} Usage: <Box flexDirection="column" paddingBottom={1}>
<ScrollArea height={6}>
{Array.from({length: 20})
.fill(true)
.map((_, index) => (
<Box key={index} flexShrink={0} borderStyle="single">
<Text>Item #{index + 1}</Text>
</Box>
))}
</ScrollArea>
</Box> CleanShot.2023-04-24.at.11.54.09.mp4I think a similar component can be made to support horizontally scrollable items as well. The problem I didn't account here is focus management. We could make However, this wouldn't work properly if we rendered a One of the options would be to explicitly document that Thinking out loud. Let me know your thoughts on this. |
This is great! Thanks for getting back to this @vadimdemedes. I just adapted your example for my old https://github.com/gnidan/ink-scroller repo and I am able to get up/down scrolling to work perfectly, with limits for when the end of content gets reached. (The I'm not super concerned with the focus-based scrolling, since I think there's a relatively straightforward solution to check for own focus, plus pass some One question, since my example ink-scroller repo uses random paragraphs of lorem-ipsum text. When I scroll left/right, Ink re-renders the paragraphs every time I scroll (i.e., it computes the word wrapping every scroll action, which makes individual lines re-wrap differently). It's possible that I just have a bug, since it's been awhile since I've looked at my example code. But in case you might know... should I just plan to add line-breaks manually and not rely on Ink's wrapping logic? |
Regarding the bounty portion of this issue, I consider the matter resolved. I can use the regular With pleasure, I'd like to remit the 1 ETH funds associated with this bounty. @developerfred: Although your solution did not meet acceptance criteria, @tin-t: I'd like to offer you a 0.2 ETH tip as gratitude for your work on the initial implementation of @vadimdemedes: Between #502 and #432 (comment), your work clearly meets the acceptance criteria, and thus you deserve the 1.0 ETH! I would like to send this to you. But, for both @tin-t and @vadimdemedes... I don't know how to issue you these payments.
I understand that the two of you might not be prepared to receive ETH. I apologize if this is the case, but unfortunately (for budgeting and tax reasons), I can't issue the payout in a different currency other than ETH. I am happy to walk you through the process of making an Ethereum account and helping you find a reputable fiat off-ramp that you can use if you want to convert the funds into your local currencies. Please let me know. For housekeeping purposes, I'll plan to leave both this GH issue and the associated bounty open until it's fully paid. I worry that something on the Gitcoin side will break otherwise... Anyway, thank you everyone who contributed to this issue, and enormous thanks to you @vadimdemedes for all your work on Ink, really :). I'm looking forward to picking up where I left off when I first got blocked by this overflow/scrolling thing. Cheers all! Edit: @vadimdemedes it looks like you might already have a Gitcoin account? But no Ethereum address associated, so I'll still wait to hear from you. |
@gnidan That's very generous of you, thank you! If you could send it either to
I think it can be solved by setting a fixed width to the |
Sent. I hope it makes a difference!
This works, thank you!
I closed the bounty on the Gitcoin side, and I'm going to close this issue now. (@tin-t it looks like you have not been active on Github in awhile, but if you see these notifications, please find a way to reach out to me so I can make good on my stated offer above! Hopefully I will see any notifications here, but if not, my username is |
Thanks @gnidan, every dollar makes the Ukrainian victory closer 💛 |
Firstly: this library is amazing; I'm so excited to use it everywhere I possibly can. Sadly, this issue describes a use case that's blocking me from moving forward :(
I'm opening this issue and offering a corresponding Gitcoin bounty in hopes it will help. Thank you to anyone who can help make this happen!
Summary
This feature request seeks to cover the use case where a component may exceed the bounds of its container. Specifically, this issue seeks the following behaviors:
<Box overflow={...}>
, perhaps as implemented by Addoverflow
prop toBox
component #393overflow
prop toBox
component #393 alone is insufficient, since it doesn't provide a way for a component to measure what its full width/height would be. (Whenoverflow="hidden"
is provided, width/height are measured based on the final component on-screen, and there seems to be no way around this)Related
PR #393 (Add
overflow
prop to Box component) is mostly complete for my concerns here. I outlined my concerns with scrolling / measurements in this comment, but no idea what the right answer is.vadimdemedes/goodness-squad#5 describes a related use case (Fullscreen UIs). This issue doesn't target fullscreen UIs specifically; overflow/scrolling behavior should work regardless of whether the component is going to be fullscreen or not.
Acceptance criteria
See comment below
Notes
Please let me know if the requirements could use clarification! I'm very keen to get this working 🙇
The text was updated successfully, but these errors were encountered: