Skip to content

Conversation

@sdinesh86
Copy link
Contributor

(...) #17495

Cleaned up the code and now making specific PR for material browser


for( var i = 0; i < materials.length; i ++ ) {

if ( materials[i].id === id ) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are some minor style issues in your code. I suggest you setup your editor in a way so it automatically respect the project's style guide (defined in mdcs).

You can then automatically fix such issues and produce outputs like:

if ( materials[ i ].id === id ) {

    material = materials[ i ];
    break;

}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do it,

Will need some getting used to, to these linters :)

@mrdoob
Copy link
Owner

mrdoob commented Sep 23, 2019

I think this is good enough. I'll clean things after merging 👌

@mrdoob mrdoob added this to the r109 milestone Sep 23, 2019
@mrdoob mrdoob merged commit b29e754 into mrdoob:dev Sep 23, 2019
@mrdoob
Copy link
Owner

mrdoob commented Sep 23, 2019

Thanks!

@mrdoob
Copy link
Owner

mrdoob commented Sep 23, 2019

@mrdoob
Copy link
Owner

mrdoob commented Sep 23, 2019

@sdinesh86 Also 3b9016b 294246b b8903a4

@sdinesh86
Copy link
Contributor Author

@mrdoob THANKS for the cleanup and update !!!

I will keep a note of these things in next PR

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 19, 2019

While testing the ES6 module code, I've encountered something I'm not sure about. It's possible to reproduce in prod:

  • Go to https://threejs.org/editor/
  • Add a Box
  • Go to "PROJECT" tab. You see the instance of MeshStandardMaterial in the materials section.
  • Go back to "SCENE" and change the box's material to MeshBasicMaterial
  • Go to "PROJECT" tab. The material browser shows MeshStandardMaterial but not the newly added MeshBasicMaterial

Is this a bug or am I doing something wrong?

Besides, when using the dark mode CSS, it's really hard to recognize the active list element:

image

The bottom list element is actually active^^.

@mrdoob
Copy link
Owner

mrdoob commented Dec 19, 2019

Is this a bug or am I doing something wrong?

It is definitely a bug. We're probably not dispatching a signal when changing the material.

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 20, 2019

I've tried to fix it via e66bcfe in #18201.

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.

3 participants