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

Improvements to Node Autocomplete UI #11663

Merged
merged 5 commits into from
May 6, 2021
Merged

Conversation

reddyashish
Copy link
Contributor

@reddyashish reddyashish commented May 4, 2021

Purpose

This PR is to make Node Autocomplete UI match the Dynamo Visual Refresh styling.
Task: https://jira.autodesk.com/browse/DYN-3359

Screen Shot 2021-05-04 at 8 37 22 AM

Screen Shot 2021-05-04 at 8 37 14 AM

Things to do:

  1. We need a new more information icon as we don't have it being used anywhere else
  2. Add a new task for implementing additional features

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated

Reviewers

@QilongTang @mjkkirschner @zeusongit @Amoursol

@Amoursol
Copy link
Contributor

Amoursol commented May 4, 2021

Hi @reddyashish - Looking pretty awesome so far! A few comments on what can be improved:

  1. It looks like the Search icon in the bar is truncated at the top and bottom (Flattened off). We probably need a little more breathing room here? [Orange]
  2. Looks like the search text is also not centre justified? Would it look better if so? Or maybe center justified and a little smaller? [Green]
  3. Looks like we have a lot of white space (Grey in this case) on the left hand margin of the results, and no white space (Grey space) on the right. Can we shift the results more to the left, and build in the same border / white space for the results? [Lilac]
  4. Can we make the "X" in the upper right white please? And also the same overall size as the "Autocomplete" header text? Also confirm they are justified the same. [Pink]

image

@Jingyi-Wen any additional thoughts here?

@Jingyi-Wen
Copy link

Looking good! I agree with @Amoursol 's comments above, also covered everything!

@reddyashish
Copy link
Contributor Author

reddyashish commented May 5, 2021

@Amoursol I have made some changes based on your comments. This is the current look:

Screen Shot 2021-05-05 at 9 27 04 AM

  1. Addressed the search icon issue.
  2. Made the search text little smaller and center-spaced(vertically).
  3. Moved the results to the left and aligned it with other components.
  4. Regarding the 'X' icon, I am actually using the image from here:https://github.com/DynamoDS/Dynamo/blob/master/src/DynamoCoreWpf/UI/Images/closetab_normal.png, which is used in other Dynamo locations. We do not have a white close icon as of now. What do you think?

@Amoursol
Copy link
Contributor

Amoursol commented May 5, 2021

  1. Addressed the search icon issue.
  2. Made the search text little smaller and center-spaced(vertically).
  3. Moved the results to the left and aligned it with other components.
  4. Regarding the 'X' icon, I am actually using the image from here:https://github.com/DynamoDS/Dynamo/blob/master/src/DynamoCoreWpf/UI/Images/closetab_normal.png, which is used in other Dynamo locations. We do not have a white close icon as of now. What do you think?

Awesome, thank you @reddyashish! Search icon and bar text looks great now.

Would it be possible to give a little more breathing room (i.e. grey/white space) for both sides of the results? It's very close, I'm thinking only a few pixels here.

@Jingyi-Wen are you able to provide a white cross Icon for this work? We don't currently have one 😊

@Jingyi-Wen
Copy link

Jingyi-Wen commented May 5, 2021

Here's a white close icon (16*16 pixels, same as current one)
closetab_normal_white

@QilongTang
Copy link
Contributor

You guys rock! I will take a look tomorrow

@Amoursol
Copy link
Contributor

Amoursol commented May 5, 2021

You guys rock! I will take a look tomorrow

Make sure you enjoy your vacay first man 🙌

@reddyashish
Copy link
Contributor Author

@Amoursol @Jingyi-Wen Addressed the suggested changes. Here is the current look:
Screen Shot 2021-05-06 at 9 25 06 AM

Let me know if you think of any changes.

@QilongTang
Copy link
Contributor

@Amoursol @Jingyi-Wen @reddyashish I found Search all nodes misleading in this case because what it really does is searching through all autocomplete suggestions, anyway we can capture that?

@reddyashish
Copy link
Contributor Author

Yes, I agree. Something like "Search Node Results"?

@Jingyi-Wen
Copy link

Agreed! I'd suggest "Search Autocomplete Results", or just "Search" would work

@QilongTang
Copy link
Contributor

QilongTang commented May 6, 2021

@reddyashish @Jingyi-Wen Nice, I vote for Search Autocomplete Results so we can be specific. Otherthan that, LGTM

@Amoursol
Copy link
Contributor

Amoursol commented May 6, 2021

LGTM with the 'Search Autocomplete Results' in the search bar amendment @reddyashish !

@reddyashish
Copy link
Contributor Author

Yes, changed it.
Screen Shot 2021-05-06 at 12 14 24 PM

Merging this PR.

@reddyashish reddyashish merged commit 08ed01b into DynamoDS:master May 6, 2021
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.

4 participants