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

Feature - inline ui can now merge OR groupings #193

Merged
merged 5 commits into from
Sep 19, 2024
Merged

Conversation

ssteffen
Copy link
Collaborator

In the inline UI, the OR groups are separated by the text "OR". This PR updates this pill to have a remove button. Upon activation, the Or conjunction is flipped to an And conjunction, thereby merging the two groups.

This PR also:

  • Adds support for stimulus tooltips by adding data attribute shortcuts
  • Addresses a few more responsive design issues with the toolbar

@ssteffen ssteffen self-assigned this Sep 18, 2024

def set_blank_filter
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This wasn't being used anywhere and was leftover from my work on the clear button. So removed

@@ -93,12 +93,17 @@ def clear
handle_filter_update()
end

private
def merge_groups
@criterion = Refine::Inline::Criterion.new(criterion_params.merge(refine_filter: @refine_filter))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Technically we don't need a criterion object to perform the operation required here, but the controller expects a criterion so the action just passes in the first element of the group being acted on. Its not ideal and arguably we should have a conjunction controller or a rethink of how criteria are managed. Today criterion are also acting on their surrounding conjunctions and it can be confusing but I didn't want to mess with this paradigm in this work

def change_conjunction(index, conjunction_word)
if conjunction_word == "and"
blueprint[index][:word] = "and"
elsif conjunction_word == "or"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No real use case for this behavior but I went ahead and added it since it was easy

@@ -15,15 +15,7 @@
<% else %>
<div class="refine--groups-wrapper">
<% groups.each.with_index do |group, i| %>
<% unless i == 0 %>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

most of this got moved into the or_separator partial

@ssteffen ssteffen merged commit 3a06595 into main Sep 19, 2024
1 check passed
@ssteffen ssteffen deleted the tooltip-design-pass branch September 19, 2024 14:32
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.

2 participants