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

✨ Controls placement on resource level #3320

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

RocKhalil
Copy link
Contributor

@RocKhalil RocKhalil commented Oct 9, 2024

Description

You can now have the placement of the actions set on the resource too. So you can have a resource with controls on the left and another one with controls on the right.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

Manual review steps

  1. On a resource level, add self.controls_placement = :left
  2. View the controls being rendered on the left instead of the default right value

Copy link

codeclimate bot commented Oct 9, 2024

Code Climate has analyzed commit e65d859 and detected 0 issues on this pull request.

View more on Code Climate.

Copy link
Contributor

@Paul-Bob Paul-Bob left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution @RocKhalil!

We should wrap this logic on a concern and remove unused methods, otherwise the logic is done well!

Comment on lines 2 to 4
<%
resource_controls_placement = @resource.controls_placement || Avo.configuration.resource_controls_placement
%>
Copy link
Contributor

@Paul-Bob Paul-Bob Oct 10, 2024

Choose a reason for hiding this comment

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

Let's extract this logic to a concern, something like

def controls_placement
  @controls_placement ||= @resource.controls_placement || Avo.configuration.resource_controls_placement
end

def resource_controls_on_the_right?
  controls_placement == :right
end

def resource_controls_on_the_left?
  controls_placement == :left
end

Then include the concern on the components that uses this logic.

Let's also ensure that the original methods resource_controls_on_the_right? and resource_controls_on_the_left? are deleted if they are no longer used.

@RocKhalil
Copy link
Contributor Author

@Paul-Bob refactored into a concern + added both so we can render the controls on both sides if someone wants it

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.

3 participants