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

Message notes cover all other messages again #541

Closed
alelom opened this issue Sep 17, 2020 · 7 comments · Fixed by #612
Closed

Message notes cover all other messages again #541

alelom opened this issue Sep 17, 2020 · 7 comments · Fixed by #612
Assignees
Labels
type:bug Error or unexpected behaviour
Projects

Comments

@alelom
Copy link
Member

alelom commented Sep 17, 2020

Description:

We have again the same behaviour described in #480
that was fixed in #490.

Steps to reproduce:

Have any grasshopper component return a Note and additionally a Warning and/or an Error.
The Note colour will prevail, as described in #480.

Expected behaviour:

Note should not cover. Same behaviour obtained thanks to #490.

Test file(s):

@alelom alelom added the type:bug Error or unexpected behaviour label Sep 17, 2020
@IsakNaslundBh
Copy link
Contributor

Think this is working as expected on my machine. Using the testmethods produced by you alessio in #490 (review)

image

@alelom
Copy link
Member Author

alelom commented Sep 17, 2020

I'm basing this on a specific case I'm having:
image
here the component should turn orange, but it's not.

Before pulling out our hair, I'll do more tests asap.

@alelom
Copy link
Member Author

alelom commented Sep 17, 2020

Doing this action in Logging.cs, as suggested by @IsakNaslundBh:
image

solved the problem for me.

@adecler adecler self-assigned this Sep 18, 2020
@adecler
Copy link
Member

adecler commented Sep 18, 2020

We have again the same behaviour described in #480
that was fixed in #490.

Checking the code on the master, it is exactly the same as it was after the fix in #490. Your own test confirms that the functionality hasn't changed.

@alelom , happy to make the change suggested but first you need to provide a test file that I can easily use to understand why your case is different than all the others.

@IsakNaslundBh , since you did the initial fix, any insight as to why the if-else is no longer relevant ? I assume it was added for a reason 😄

@IsakNaslundBh
Copy link
Contributor

IsakNaslundBh commented Sep 18, 2020

@alelom , happy to make the change suggested but first you need to provide a test file that I can easily use to understand why your case is different than all the others.

My thinking was that @alelom s case reached the logging method multiple times, so some notes had time to get added as remarks instead of blanks, so completely removing the If statement and only adding blanks with solve it.

@IsakNaslundBh , since you did the initial fix, any insight as to why the if-else is no longer relevant ? I assume it was added for a reason 😄

Forgot the reason yesterday, when I pinged the solution to alessio, but remember again today why it is done in that not so clean way. If a component only contains blank messages, it will not trigger the display of the bubble in the upper right corner, and not trigger the adding of remarks to the menu... So my suggested solution is no good.

image

Gah, only thing I can think of then is to check the component for any messages. Clear them and re-add them every time we add events. All for the sake of a faulty enum in GH...

@adecler adecler added this to Backlog in UI Priority via automation Sep 29, 2020
@adecler adecler moved this from Backlog to Priority 5 in UI Priority Sep 29, 2020
@adecler
Copy link
Member

adecler commented Feb 23, 2021

@alelom , can you provide an example script that reproduces the bug ?

I have tested this with both the example you provide in #490 (review) and the Code Editor and it works as expected.

image

@alelom
Copy link
Member Author

alelom commented Feb 23, 2021

Please find below

https://burohappold.sharepoint.com/:u:/s/BHoM/EaLHWrCr9NVBoz76ubs8DKwBoU2QMt6dd6j4IE8EsbRoug?e=rYww0U

It's the last Execute component on the right.
image

UI Priority automation moved this from Priority 5 to Completed Feb 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Error or unexpected behaviour
Projects
UI Priority
  
Completed
Development

Successfully merging a pull request may close this issue.

3 participants