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

Focus control upstream #1173

Closed
wants to merge 24 commits into from
Closed

Focus control upstream #1173

wants to merge 24 commits into from

Conversation

meguiraun
Copy link
Contributor

hi guys,

This is an attempt to bring Biomax's old focus control element into latest mxcube.

WIP because your input is needed on how we handle these kind of components, it technically works as it is.

  • Added a new BeamFocus component, it currently has Biomax specific logic but i did not want to generalise too much too early
  • Configurable "beam control" elements in the ui
Screenshot 2024-03-04 at 08 48 34

ui/package.json Outdated Show resolved Hide resolved
@marcus-oscarsson
Copy link
Member

Just a question, how is this control used, is to de-focus the beam to change the beam size ?

@meguiraun
Copy link
Contributor Author

Just a question, how is this control used, is to de-focus the beam to change the beam size ?

The user selects a beam size appropriate for their crystal size, which corresponds to different beam (de)focus

@meguiraun
Copy link
Contributor Author

meguiraun commented Mar 15, 2024

I refactored few things here and there. In essence I added the beam definer information into the beamInfo info dict, and propagate it to the UI.

I based on the ID232 beam definer approach as NState hwobj, and adapted from my previous attempt, now "100x100" is one option in the definer object.

One thing that I have not solved is how to generalize the UI styling (see this ), any suggestion is welcome

I also created a new beam definer mockup:
mxcube/mxcubecore#872

beam_definer

@meguiraun meguiraun changed the title WIP: Focus control upstream Focus control upstream Mar 21, 2024
@meguiraun
Copy link
Contributor Author

I moved down into the hwobj the custom css styling.

please, have a look!

Copy link
Collaborator

@axelboc axelboc left a comment

Choose a reason for hiding this comment

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

Looking great! I've got lots of suggestions but don't feel like you have to resolve them — it's all coding style, nothing major.

ui/src/components/SampleView/BeamDefinerInput.js Outdated Show resolved Hide resolved
ui/src/components/SampleView/BeamDefinerInput.js Outdated Show resolved Hide resolved
<BeamDefinerInput
beamDefinerInputList={this.props.sampleViewState.definerList}
currentDefiner={this.props.sampleViewState.currentDefiner}
customStyling={this.props.sampleViewState.customStyling}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like the logic in the render method of BeamDefinerInput would be much simpler if you passed this through the apertureList prop:

apertureList={this.props.sampleViewState.customStyling || this.props.sampleViewState.apertureList}

This way, you can have a single loop to create the optionList array (you may not even need this optionList array at all, since the loop below can be moved directly into the JSX):

const optionList = Object.entries(this.props.apertureList).map((key, value) => {
  // key is the label (e.g. "50x50")
  // value is either the same as the label or an object when `customStyling` is defined, so you can do:
  if (typeof value === 'object') {
    return { ... }; // or return a `<Dropdown.Item />` directly if you move the loop inside the JSX
  }
  return { ... }
});

ui/src/containers/SampleViewContainer.js Outdated Show resolved Hide resolved
@marcus-oscarsson
Copy link
Member

Just a status check, where are we with this one @meguiraun ?

@meguiraun meguiraun closed this Oct 24, 2024
@fabcor-maxiv
Copy link
Contributor

@marcus-oscarsson @meguiraun Is it okay to delete the branch?

@meguiraun meguiraun deleted the focus_control_upstream branch October 24, 2024 09:59
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