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

Member index: controls layout refactor #17927

Merged
merged 6 commits into from
Jun 17, 2022
Merged

Conversation

aitchiss
Copy link
Contributor

@aitchiss aitchiss commented Jun 15, 2022

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

This PR shuffles around the alignment and position of the "controls" on /admin/member_manager/users. This reshuffle is behind the member_index_view feature flag, and the old code can/should be removed when we ship phase 2.

I decided in the end to place it behind the feature flag to avoid complexity with including the "filter by role" view on the index page vs in the new filters modal. Having it behind the feature flag will make it more straightforward to extricate the old code.

Related Tickets & Documents

QA Instructions, Screenshots, Recordings

All changes are contained within /admin/member_manager/users.

  • With the member_index_view feature flag disabled, you should see no change to the current UI. Filtering by role and searching should continue to work as expected. Expandable search/filter functionality on smaller screens should continue to function
  • With the member_index_view feature flag enabled, the UI should match the breakpoints detailed in Figma, the search functionality should continue to work as expected, expandable search section on small screens should continue to work, and the filter button should pop open the new filters modal
CleanShot.2022-06-15.at.14.11.52.mp4

UI accessibility concerns?

None, just reshuffling elements

Added/updated tests?

  • Yes
  • No, and this is why: existing tests continue to cover us
  • I need help with writing tests

[Forem core team only] How will this change be communicated?

Will this PR introduce a change that impacts Forem members or creators, the
development process, or any of our internal teams? If so, please note how you
will share this change with the people who need to know about it.

  • I've updated the Developer Docs or
    Storybook (for Crayons components)
  • This PR changes the Forem platform and our documentation needs to be
    updated. I have filled out the
    Changes Requested
    issue template so Community Success can help update the Admin Docs
    appropriately.
  • I've updated the README or added inline documentation
  • I've added an entry to
    CHANGELOG.md
  • I will share this change in a Changelog
    or in a forem.dev post
  • I will share this change internally with the appropriate teams
  • I'm not sure how best to communicate this change and need help
  • This change does not need to be communicated, and this is why not: still behind a feature flag

[optional] What gif best describes this PR or how it makes you feel?

real life tetris

@pr-triage pr-triage bot added the PR: draft bot applied label for PR's that are a work in progress label Jun 15, 2022
@@ -1,6 +1,36 @@
<%= javascript_packs_with_chunks_tag "admin/users/controls", defer: true %>
<% if params[:controller] == "admin/users" %>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We didn't need this, since we only use this partial on the member index page 🤷‍♀️ I think perhaps we missed removing it in a refactor at some point

@aitchiss aitchiss marked this pull request as ready for review June 15, 2022 13:51
@aitchiss aitchiss requested a review from a team June 15, 2022 13:51
@pr-triage pr-triage bot added PR: unreviewed bot applied label for PR's with no review and removed PR: draft bot applied label for PR's that are a work in progress labels Jun 15, 2022
@aitchiss aitchiss requested review from a team, Ridhwana and msarit and removed request for a team June 15, 2022 13:51
@aitchiss aitchiss self-assigned this Jun 15, 2022
Copy link
Contributor

@fdocr fdocr left a comment

Choose a reason for hiding this comment

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

Tried it out and works+looks nice!

@pr-triage pr-triage bot added PR: partially-approved bot applied label for PR's where a single reviewer approves changes and removed PR: unreviewed bot applied label for PR's with no review labels Jun 15, 2022
Copy link
Contributor

@jaw6 jaw6 left a comment

Choose a reason for hiding this comment

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

This looks good to me! 👍

@aitchiss aitchiss merged commit 48818a9 into main Jun 17, 2022
@aitchiss aitchiss deleted the aitchiss/controls-ui-refactor branch June 17, 2022 09:33
@pr-triage pr-triage bot added PR: merged bot applied label for PR's that are merged and removed PR: partially-approved bot applied label for PR's where a single reviewer approves changes labels Jun 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: merged bot applied label for PR's that are merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Member index view filters: Update controls layouts
3 participants