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

DrawPanel Update: Repositioning the mode_panel and mode_text #678

Merged
merged 8 commits into from Sep 9, 2022

Conversation

ganimtron-10
Copy link
Contributor

Whenever we create, translate, or rotate the shapes on the panel it sometimes overlaps the mode_panel or mode_text which are used to select and show the current mode respectively.
eg.
python_jHAyyCrRD2

Repositioning the mode_panel and the mode_text to be outside the panel to solve the overlapping issue.
eg.
python_OZovev32PG

Copy link
Contributor

@skoudoro skoudoro left a comment

Choose a reason for hiding this comment

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

Hi @ganimtron-10,

Indeed, it is better like this.

You might need to record again all your tests because all DrawPanel tests are failing due to recent changes. You can easily check it locally

Let me know when it is updated and ready to be reviewed

@ganimtron-10
Copy link
Contributor Author

Hello @skoudoro ,
I have updated the tests and fixed the slider bug.
If all test passes then it would be ready to go.

@skoudoro
Copy link
Contributor

skoudoro commented Sep 6, 2022

As you can see on the failing tests, there are still some issues. Please, check them. see below an error example

self = <fury.ui.elements.DrawShape object at 0x7f6ef0027290>

    def _setup(self):
        """Setup this UI component.
    
        Create a Shape.
        """
        if self.shape_type == "line":
            self.shape = Rectangle2D(size=(3, 3))
        elif self.shape_type == "quad":
            self.shape = Rectangle2D(size=(3, 3))
        elif self.shape_type == "circle":
            self.shape = Disk2D(outer_radius=2)
        else:
            raise IOError("Unknown shape type: {}.".format(self.shape_type))
    
        self.shape.on_left_mouse_button_pressed = self.left_button_pressed
        self.shape.on_left_mouse_button_dragged = self.left_button_dragged
        self.shape.on_left_mouse_button_released = self.left_button_released
    
        self.rotation_slider = RingSlider2D(initial_value=0,
                                            text_template="{angle:5.1f}°")
        self.rotation_slider.set_visibility(False)
>       slider_position = self.drawpanel.canvas.position + \
            [self.drawpanel.canvas.size[0] - self.rotation_slider.size[0]/2,
             self.rotation_slider.size[1]/2]
E       AttributeError: 'NoneType' object has no attribute 'canvas'

@ganimtron-10
Copy link
Contributor Author

For now, I am just adding a check to see whether the DrawPanel is available or not.
I will create a follow-up PR in which I will move the rotation slider to the draw panel as discussed in the meeting.

@codecov
Copy link

codecov bot commented Sep 9, 2022

Codecov Report

Merging #678 (d9f1363) into master (a8f1d76) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #678      +/-   ##
==========================================
- Coverage   50.38%   50.37%   -0.01%     
==========================================
  Files         120      120              
  Lines       27014    27082      +68     
  Branches     2986     2994       +8     
==========================================
+ Hits        13610    13642      +32     
- Misses      12945    12979      +34     
- Partials      459      461       +2     
Impacted Files Coverage Δ
fury/ui/elements.py 0.00% <0.00%> (ø)
fury/ui/tests/test_elements.py 0.00% <0.00%> (ø)
fury/fury/ui/helpers.py 96.34% <0.00%> (-1.88%) ⬇️
fury/fury/ui/elements.py 90.02% <0.00%> (-0.09%) ⬇️
fury/ui/helpers.py 0.00% <0.00%> (ø)
fury/ui/tests/test_helpers.py 0.00% <0.00%> (ø)
fury/fury/ui/tests/test_helpers.py 100.00% <0.00%> (ø)

@skoudoro
Copy link
Contributor

skoudoro commented Sep 9, 2022

Sounds like a plan. Looking forward to seeing your next PR. For now, I am going ahead and merging this PR. it looks ok.

Thanks @ganimtron-10

@skoudoro skoudoro merged commit 116762e into fury-gl:master Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants