Several bug fixes and improvements of `SmartNodeSelector` #178

rubenthoms · 2021-10-13T15:08:52Z

Feature: OR operator in nodes (beta)
Bug fix: Placeholder not applied
Bug fix: Text width not calculated correctly initially
Feature: Case-insensitive search and apply (optional feature)
Feature: Export of data types
Feature: Search on descriptions
Bug fix: Clicking enter while node editing not finished jumped to next node (if available)
Improved and bug fixed: Arrow navigation within node input field
Feature: Tab navigation
Feature: End/Home navigation
Improved visual feedback
Ctrl+Space always opens all suggestions
Bug fix: Incorrect positioning when scrolling
Bug fix: Minus was not allowed in node names

Closes #149.
Closes #97.
Fixes #186.
Closes #188.
Addresses #128.

anders-kiaer

Nice! 🚀🐞🐛

CHANGELOG.md

jorgenherje

Tested and looks very nice! Some comments.

Seem to have issue with wildcard-functionality

Comments:

OR operator only working on last node, gives issues otherwise.
Export of data types: Should additional types be exported?
Would be nice to test fixes in VectorSelector usage, but now a local pack of wcc and build of wsc does not work, as wsc is not updated with regards to the fixes in SmartNodeSelector
Having issues with wildcard operator "*". When using wildcard, the nodes does not seem to be selected.

jorgenherje · 2021-10-20T10:38:56Z

react/src/lib/components/SmartNodeSelector/components/SmartNodeSelectorComponent.tsx

+        }
+
+        const prohibitedDelimiters = ["|"];
+        if (prohibitedDelimiters.includes(props.delimiter)) {


Should "|" always be prohibited, or should it only be prohibited when OR-operator is used - i.e. when useBetaFeatuers = true?

I see your point. The intention with a prohibited char was to guarantee consistent behaviour. No matter which allowed delimiter is assigned, one can always turn on or off the beta features without being confronted with sudden error messages. On the other hand, it is not quite appreciable yet how much the beta features are going to be used (or if they are going to be used at all). So there is the downside of restricting the component user (a bit). IMO, restricting the component user to not using one certain single char is not too much of a downside compared to the potential downside of an inconsistency (however, both downsides are rather minor). What do you think?

I agree with your point. Both easier and safer to prioritize consistent behavior by always prohibit the OR-operator, rather than wanting to use "|" as a delimiter.

jorgenherje

Really nice features and fixes! 👍

Tested and verified!

jorgenherje

Tested and found some issues.

Testing with usage in VectorSelector (local build) in webviz-subsurface plugin.
Note: VectorSelector in wsc must be updated with showAllSuggestions prop

Issues:

Scrolling in suggestions not working when Ctrl+space. If exiting the node edit and entering again, the scroll works.
When Ctrl+space shows all suggestions, the showAllSuggestions setting remains active when re-entering node editing. I.e. when entering node, matching suggestions are shown in list. When Ctrl+space is entered all suggestions are shown. If one exit (push outside of the editing node in GUI) and enter the node edit again, all suggestions are still shown.

rubenthoms · 2021-11-08T13:06:40Z

See also #188.

jorgenherje

Tested and found some issues:

When entering node level, the filtered search removes non-matching suggestions, but then they reappear after a fraction of a second (without pushing Ctrl+space):
The suggestions does not scroll to active/marked suggestion:

- Bug fix: Suggestions window not shown correctly when scrolling - Bug fix: Crash when switching between tabs in Webviz plugin - Feature: Ctrl+Space opens all suggestions - Bug fix: "-" was not allowed in node names

Also added no-suggestions-placeholder

jorgenherje

Tested and found issues:

Ctrl+space stopped working?
Remove debug code

react/src/lib/components/SmartNodeSelector/components/SmartNodeSelectorComponent.tsx

- Wildcards not supported in `findSuggestions` function - Mousedown on suggestion blurred input field -> unexpected behaviour - Added shaking timer property to securely remove it on unmount

jorgenherje

Tested functionality and reviewed code changes. Really nice features! 👍

rubenthoms self-assigned this Oct 13, 2021

rubenthoms marked this pull request as ready for review October 14, 2021 11:10

rubenthoms requested review from anders-kiaer and asnyv October 14, 2021 11:10

rubenthoms force-pushed the EQ_149-SmartNodeSelector-improvements-and-fixes branch from 986132d to 6df3f16 Compare October 18, 2021 11:18

anders-kiaer requested changes Oct 18, 2021

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

rubenthoms requested review from anders-kiaer and jorgenherje October 19, 2021 11:37

anders-kiaer approved these changes Oct 19, 2021

View reviewed changes

jorgenherje reviewed Oct 20, 2021

View reviewed changes

jorgenherje mentioned this pull request Oct 20, 2021

Minor updates of VectorCalculator based on SmartNodeSelector enhancement equinor/webviz-subsurface-components#608

Closed

anders-kiaer added bug Something isn't working enhancement New feature or request next release 🚢 To be included in next release labels Oct 22, 2021

rubenthoms removed the next release 🚢 To be included in next release label Oct 22, 2021

rubenthoms force-pushed the EQ_149-SmartNodeSelector-improvements-and-fixes branch from 7d4fcef to 449d0a9 Compare October 26, 2021 15:12

rubenthoms requested review from jorgenherje and anders-kiaer October 26, 2021 15:13

rubenthoms mentioned this pull request Oct 29, 2021

Implemented new SmartNodeSelector features in VectorSelector and fixed bugs equinor/webviz-subsurface-components#626

Merged

jorgenherje approved these changes Nov 3, 2021

View reviewed changes

rubenthoms requested a review from jorgenherje November 4, 2021 16:16

anders-kiaer added the next release 🚢 To be included in next release label Nov 8, 2021

jorgenherje requested changes Nov 8, 2021

View reviewed changes

tnatt mentioned this pull request Nov 8, 2021

SmartNodeSelector issues after testing #188

Closed

2 tasks

rubenthoms requested a review from jorgenherje November 9, 2021 09:30

jorgenherje requested changes Nov 9, 2021

View reviewed changes

rubenthoms added 6 commits November 9, 2021 14:49

More consistent home/end tag selection.

327059c

Several fixes and improvements

ad1b7b7

- Bug fix: Suggestions window not shown correctly when scrolling - Bug fix: Crash when switching between tabs in Webviz plugin - Feature: Ctrl+Space opens all suggestions - Bug fix: "-" was not allowed in node names

Several bug fixes and improvements

9005053

Adjusted snapshot file.

8c7ee3c

Suggestions window wrongly positioned when scrolling.

b75a9ef

Adjusted changelog, fixed issue showing all suggestions after editing

ae21518

rubenthoms force-pushed the EQ_149-SmartNodeSelector-improvements-and-fixes branch from 3635b38 to ae21518 Compare November 9, 2021 13:49

rubenthoms requested a review from jorgenherje November 9, 2021 13:49

Fixed bug setting the selected suggestion at wrong node level

b742cd3

Also added no-suggestions-placeholder

jorgenherje requested changes Nov 9, 2021

View reviewed changes

react/src/lib/components/SmartNodeSelector/components/SmartNodeSelectorComponent.tsx Outdated Show resolved Hide resolved

rubenthoms added 11 commits November 9, 2021 16:17

Bug fix: Ctrl+Space not working anymore

65b5570

Removed debug code.

48375e7

Bug fix: possible crash when navigating with arrows

e6708f1

Improved styling and added exports.

bde036c

Prevent default behaviour when deleting selected tags.

95eb58d

Several bug fixes

7cdb481

- Wildcards not supported in `findSuggestions` function - Mousedown on suggestion blurred input field -> unexpected behaviour - Added shaking timer property to securely remove it on unmount

Adjusted snapshot.

667796f

Adjusted formatting.

88bdcb0

Next iteration.

9d59a8b

Several bug fixes/iimprovements.

5a4a534

Removed debug output and added rerender when unselecting tags.

b5f4808

rubenthoms requested a review from jorgenherje November 17, 2021 13:12

rubenthoms added 3 commits November 17, 2021 14:29

Adjusted for initial update.

9e58f76

Added title to suggestions

e7af2cc

Bug fixes: Delete button and ArrowRight behaviour (when selected)

8b5c940

jorgenherje approved these changes Nov 18, 2021

View reviewed changes

rubenthoms merged commit 6e7eb48 into equinor:master Nov 18, 2021

rubenthoms mentioned this pull request Jul 13, 2022

Option to add multiple values of the same attribute in the SmartNodeSelector #128

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Several bug fixes and improvements of `SmartNodeSelector` #178

Several bug fixes and improvements of `SmartNodeSelector` #178

rubenthoms commented Oct 13, 2021 •

edited

Loading

anders-kiaer left a comment

jorgenherje left a comment

jorgenherje Oct 20, 2021

rubenthoms Oct 25, 2021

jorgenherje Oct 25, 2021

jorgenherje left a comment

jorgenherje left a comment

rubenthoms commented Nov 8, 2021

jorgenherje left a comment

jorgenherje left a comment

jorgenherje left a comment

Several bug fixes and improvements of SmartNodeSelector #178

Several bug fixes and improvements of SmartNodeSelector #178

Conversation

rubenthoms commented Oct 13, 2021 • edited Loading

anders-kiaer left a comment

Choose a reason for hiding this comment

jorgenherje left a comment

Choose a reason for hiding this comment

jorgenherje Oct 20, 2021

Choose a reason for hiding this comment

rubenthoms Oct 25, 2021

Choose a reason for hiding this comment

jorgenherje Oct 25, 2021

Choose a reason for hiding this comment

jorgenherje left a comment

Choose a reason for hiding this comment

jorgenherje left a comment

Choose a reason for hiding this comment

rubenthoms commented Nov 8, 2021

jorgenherje left a comment

Choose a reason for hiding this comment

jorgenherje left a comment

Choose a reason for hiding this comment

jorgenherje left a comment

Choose a reason for hiding this comment

Several bug fixes and improvements of `SmartNodeSelector` #178

Several bug fixes and improvements of `SmartNodeSelector` #178

rubenthoms commented Oct 13, 2021 •

edited

Loading