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

Move "array_editor" to "StandardEditors" contributed examples #1691

Merged
merged 7 commits into from Jul 22, 2021

Conversation

aaronayres35
Copy link
Contributor

@aaronayres35 aaronayres35 commented Jun 18, 2021

closes #1646

This PR takes the example array_eitor.py which used to sit in the documentation and moves it into the traitsui/examples/demo/Standard_Editors module. It also generates a new screenshot for the example and then uses these in the documentation. It removes the old screenshot and example file.

Checklist

  • Add a news fragment if this PR is news-worthy for end users. (see docs/releases/README.rst)

Copy link
Contributor

@rahulporuri rahulporuri left a comment

Choose a reason for hiding this comment

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

It looks like there are a couple of bugs here.

  • the separators between the different sections of the UI seem off. They look off in the image and they look off locally when i run the example.
  • the integer sections of the editor used to show 0 earlier but now they show 0.0.

@aaronayres35
Copy link
Contributor Author

aaronayres35 commented Jun 21, 2021

It looks like there are a couple of bugs here.

  • the separators between the different sections of the UI seem off. They look off in the image and they look off locally when i run the example.
  • the integer sections of the editor used to show 0 earlier but now they show 0.0.

Yeah very true, it doesn't look right. I believe these are separate issues / bugs from the changes here. I will open separate issues / PRs to address them.
ref:
for 2nd bullet #1694

I recall seeing other issues with the separators not lining up correctly... I will see if I can find the other issue.
EDIT: #1312 (doesn't appear to be the same issue, but related)
also relevant code I believe is what we touched in #1549

@aaronayres35 aaronayres35 mentioned this pull request Jun 24, 2021
1 task
@aaronayres35
Copy link
Contributor Author

Once #1702 is merged I will regenerate the screenshots and this PR should be good to go

@rahulporuri
Copy link
Contributor

@aaronayres35 i'm still not conviced about #1702 but I don't want this PR stuck in limbo.

Can you do some git magic locally to generate a screenshot without the separator issue - and push that screenshot on this branch? That way, we can push this PR ahead without being blocked on #1702 .

@aaronayres35
Copy link
Contributor Author

@aaronayres35 i'm still not conviced about #1702 but I don't want this PR stuck in limbo.

Can you do some git magic locally to generate a screenshot without the separator issue - and push that screenshot on this branch? That way, we can push this PR ahead without being blocked on #1702 .

That makes sense to me. TBH I am a little unsure on 1702 still myself. I don't really understand how previously things all looked correct for me locally, and now there was an off by one error (I think change occurred after updating macOS, but that doesn't mean change is correct as it might break code for older macOS?)
I've regenerated the correct looking screenshot on this PR and added a news fragment. This look okay @rahulporuri

Copy link
Contributor

@rahulporuri rahulporuri left a comment

Choose a reason for hiding this comment

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

LGTM 🎉 Thanks for the changes!

@aaronayres35 aaronayres35 merged commit c85f86b into master Jul 22, 2021
@aaronayres35 aaronayres35 deleted the ArrayEditor_demo branch July 22, 2021 10:53
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.

Move "array_editor" to "StandardEditors" contributed examples
2 participants