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

Some drop down fixes #10337

Merged
merged 5 commits into from
Jun 21, 2024
Merged

Some drop down fixes #10337

merged 5 commits into from
Jun 21, 2024

Conversation

farmaazon
Copy link
Contributor

Pull Request Description

Fixes #10185

Fixed two bugs from above issue: multiple arrows and being unable to change value from numeric input. The first was just lack of implementation; the second was caused by WidgetNumeric not handling its edit handler properly - the numeric input blur was finishing edit before click at entry could be interpreted.

Important Notes

There is still one issue with drop-down filtering; will file a bug report soon.

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,
    TypeScript,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • Unit tests have been written where possible.

@farmaazon farmaazon added --bug Type: bug -gui labels Jun 21, 2024
@farmaazon farmaazon self-assigned this Jun 21, 2024
@@ -318,6 +339,7 @@ function toggleVectorValue(vector: Ast.MutableVector, value: string, previousSta
function expressionTagClicked(tag: ExpressionTag, previousState: boolean) {
const edit = graph.startEdit()
const tagValue = resolveTagExpression(edit, tag)
console.log('expressionTagClicked', tagValue)
Copy link
Contributor

Choose a reason for hiding this comment

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

Debug logging (here and 1 below)

}),
)

watch(
() => isHovered.value && !suppressArrow.value,
Copy link
Contributor

Choose a reason for hiding this comment

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

Putting this in a computed showArrow to use here and in the v-if/v-else-if would ensure that if our logic for when to show the arrow changes, it is still consistent with when we inhibit a parent arrow.

@@ -12,6 +12,10 @@ interface SelectionArrowInfo {
/** Whether or not the arrow provided by this context instance was already requested.
* Do not request the arrow twice, it will be stolen from other elements! */
handled: boolean
/**
*
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing docs

@farmaazon farmaazon added the CI: Ready to merge This PR is eligible for automatic merge label Jun 21, 2024
@mergify mergify bot merged commit bcbdda5 into develop Jun 21, 2024
34 of 35 checks passed
@mergify mergify bot deleted the wip/farmaazon/fix-drop-down branch June 21, 2024 15:14
mergify bot pushed a commit that referenced this pull request Sep 16, 2024
Fixes #11063

The blur on Numeric Input was removed in #10337 and then reintroduced in #10512. I guess it was to handle `tab` properly, so I added another way of handling `tab` and removed blur again (with explanation).
jdunkerley pushed a commit that referenced this pull request Sep 16, 2024
Fixes #11063

The blur on Numeric Input was removed in #10337 and then reintroduced in #10512. I guess it was to handle `tab` properly, so I added another way of handling `tab` and removed blur again (with explanation).

(cherry picked from commit 53e1015)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--bug Type: bug -gui CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filter component's drop down bugs
2 participants