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.

Sign up for GitHub

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

Jump to bottom

731: Fix attributes #762

Merged
merged 13 commits into from
Jun 8, 2020
Merged

731: Fix attributes #762

merged 13 commits into from
Jun 8, 2020

Conversation

DolicaAkelloEgwel
Copy link
Contributor

@DolicaAkelloEgwel DolicaAkelloEgwel commented Jun 4, 2020 •
edited
Loading

Issue

Makes progress on #731

Description of work

Attributes working in Component fields and the Transform distance values in the case of scalar, array, and stream.

I also discovered some pixel tests that were still disabled, so I've got them working again.

Acceptance Criteria

Check that adding and editing attributes works. Also check that the pixel behaviour is still working as before.

UI tests

n/a

Nominate for Group Code Review

  • Nominate for code review

@DolicaAkelloEgwel DolicaAkelloEgwel marked this pull request as ready for review June 5, 2020 17:07
matthew-d-jones
matthew-d-jones suggested changes Jun 8, 2020
View reviewed changes
Copy link
Contributor

@matthew-d-jones matthew-d-jones left a comment

Choose a reason for hiding this comment

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

There are a few commented lines of code in tests.
If they are things not working with the new model yet, could you add "Disabled whilst working on model change" to the comment and make sure there is a ticket for the relevant change please?

tests/ui_tests/test_ui_add_component_window.py Outdated Show resolved Hide resolved
tests/ui_tests/test_ui_add_component_window.py Outdated Show resolved Hide resolved
tests/ui_tests/test_ui_add_component_window.py Outdated Show resolved Hide resolved
tests/ui_tests/test_ui_add_component_window.py Show resolved Hide resolved
matthew-d-jones
matthew-d-jones approved these changes Jun 8, 2020
View reviewed changes
Copy link
Contributor

@matthew-d-jones matthew-d-jones left a comment

Choose a reason for hiding this comment

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

Code and UI behaviour look good to me now. Can be merged if Jenkins passes.

@DolicaAkelloEgwel DolicaAkelloEgwel merged commit d2336c6 into model_change Jun 8, 2020
@DolicaAkelloEgwel DolicaAkelloEgwel deleted the 731_fix_attributes branch June 12, 2020 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthew-d-jones matthew-d-jones matthew-d-jones approved these changes

@rerpha rerpha Awaiting requested review from rerpha

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DolicaAkelloEgwel @matthew-d-jones