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

Text input widget #8873

Merged
merged 16 commits into from
Jan 30, 2024
Merged

Text input widget #8873

merged 16 commits into from
Jan 30, 2024

Conversation

vitvakatu
Copy link
Contributor

Pull Request Description

Closes #8823

text.editing.mp4

Adds a basic text widget for text literals.

Important Notes

Several known restrictions:

  • Separators would always be replaced with single quotation marks. All types of separators in Enso are supported though, and they would be correctly escaped if needed.
  • Logic for widget selection probably needs refinement (works for text literals and Text types, but does not work for Text | Integer, for example)
  • (!) There is a very annoying issue when the input field suddenly loses focus, closing the editing mode and discarding any changes. Debugging shows that it happens when we receive an engine update (and probably recreate the node component/widget tree (???)). It requires a separate investigation.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

@vitvakatu vitvakatu added CI: No changelog needed Do not require a changelog entry for this PR. -gui labels Jan 26, 2024
@vitvakatu vitvakatu self-assigned this Jan 26, 2024
app/gui2/src/components/GraphEditor/widgets/WidgetText.vue Outdated Show resolved Hide resolved
app/gui2/src/components/widgets/NumericInputWidget.vue Outdated Show resolved Hide resolved
Comment on lines 15 to 21
const editedValue = ref(props.modelValue)
watch(
() => props.modelValue,
(newValue) => {
editedValue.value = newValue
},
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as in NumericInput wouldn't hurt, I think.

app/gui2/src/components/widgets/TextInputWidget.vue Outdated Show resolved Hide resolved
app/gui2/src/components/widgets/TextInputWidget.vue Outdated Show resolved Hide resolved
app/gui2/src/util/ast/__tests__/abstract.test.ts Outdated Show resolved Hide resolved
app/gui2/src/util/ast/abstract.ts Outdated Show resolved Hide resolved
Comment on lines 22 to 24
/** Helper function to get text width to make sure that labels on the x axis do not overlap,
* and keeps it readable. */
export function getTextWidthByFont(text: string | null | undefined, font: string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Update docs in this and getTextWidthByFont First, they should differ a bit, second it's no longer used for axes only.

Copy link
Contributor

Choose a reason for hiding this comment

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

I always assumed widgets in this directory are a "general-usage" widgets. Now, this one is very specific: for editing Enso text literals (not just any text). So I think it could be merged with WidgetText, or renamed to EnsoTextInputWidget.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’m not keen on merging it with WidgetText, at least yet. So I just renamed it.

@vitvakatu vitvakatu added the CI: Ready to merge This PR is eligible for automatic merge label Jan 30, 2024
@mwu-tow mwu-tow merged commit ad6348a into develop Jan 30, 2024
22 of 28 checks passed
@mwu-tow mwu-tow deleted the wip/vitvakatu/text-input branch January 30, 2024 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-gui CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a very basic "text editor" widget, a copy of integer widget allowing any text
3 participants