Skip to content
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

Feature/multi line tool tip #1473

Merged
merged 1 commit into from
Dec 27, 2024
Merged

Feature/multi line tool tip #1473

merged 1 commit into from
Dec 27, 2024

Conversation

oscar139
Copy link
Contributor

Changes allow the Tool Tip Class to support tool-tips with multiple lines.

Copy link
Member

@DanRStevens DanRStevens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about the impact to the API of this change. Do we want to use a std::vector of strings, or do we want to support strings with embedded newlines? I'd be tempted to consider strings with embedded newlines. Seems like a more natural API to use. Particularly since sources of multi-line input may come with naturally embedded newlines.

@oscar139 oscar139 force-pushed the Feature/Multi-line-tool-tip branch 2 times, most recently from f3d5c6e to 02dfa90 Compare December 20, 2024 18:11
libControls/ToolTip.cpp Outdated Show resolved Hide resolved
libControls/ToolTip.cpp Outdated Show resolved Hide resolved
libControls/ToolTip.cpp Outdated Show resolved Hide resolved
@DanRStevens
Copy link
Member

This might be of interest to a couple of those comments:
std::getline

    // read file line by line
    std::istringstream input;
    input.str("1\n2\n3\n4\n5\n6\n7\n");
    int sum = 0;
    for (std::string line; std::getline(input, line);)
        sum += std::stoi(line);
    std::cout << "\nThe sum is " << sum << ".\n\n";

The use of the for loop is interesting.

@oscar139 oscar139 force-pushed the Feature/Multi-line-tool-tip branch from 02dfa90 to 61bc8bd Compare December 21, 2024 20:55
Copy link
Member

@DanRStevens DanRStevens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm kind of liking how processLines cleans up some of that earlier duplication.

libControls/ToolTip.cpp Outdated Show resolved Hide resolved
libControls/ToolTip.cpp Outdated Show resolved Hide resolved
libControls/ToolTip.h Outdated Show resolved Hide resolved
libControls/ToolTip.cpp Outdated Show resolved Hide resolved
@oscar139 oscar139 force-pushed the Feature/Multi-line-tool-tip branch 5 times, most recently from c88d306 to ecd45f6 Compare December 22, 2024 15:22
Copy link
Member

@DanRStevens DanRStevens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm quite happy with this. Had a couple of small optional thoughts.

libControls/ToolTip.cpp Outdated Show resolved Hide resolved
libControls/ToolTip.cpp Outdated Show resolved Hide resolved
@DanRStevens DanRStevens force-pushed the Feature/Multi-line-tool-tip branch from ecd45f6 to edff544 Compare December 26, 2024 23:23
@oscar139 oscar139 force-pushed the Feature/Multi-line-tool-tip branch from edff544 to fd8bdd2 Compare December 27, 2024 19:31
Comment on lines +135 to +138
forEachLine(mFocusedControl->second, [this, &renderer, &linePosition](const std::string& lineStr) {
renderer.drawText(mFont, lineStr, linePosition);
linePosition.y += mFont.height();
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also possible to do a named capture, as an alternative to capturing this, and everything that has access to:

[&font=mFont, &renderer, &linePosition]

@DanRStevens DanRStevens merged commit 9e9791c into main Dec 27, 2024
7 checks passed
@DanRStevens DanRStevens deleted the Feature/Multi-line-tool-tip branch December 27, 2024 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants