Skip to content

Redesign: fix assembly members sidebar menu#11699

Merged
andreslucena merged 1 commit intodevelopfrom
fix/assembly-members-menu
Oct 4, 2023
Merged

Redesign: fix assembly members sidebar menu#11699
andreslucena merged 1 commit intodevelopfrom
fix/assembly-members-menu

Conversation

@alecslupu
Copy link
Copy Markdown
Contributor

@alecslupu alecslupu commented Oct 2, 2023

🎩 What? Why?

This PR adds Assembly sidebar to the Assembly member page.

📌 Related Issues

Link your PR to an issue

Testing

  1. Login as admin and visit assembly member admin area
  2. Apply patch
  3. See there is a Sidebar menu

📷 Screenshots

Please add screenshots of the changes you are proposing
Before:
image

After:
image

♥️ Thank you!

@alecslupu alecslupu added module: assemblies project: redesign Barcelona City Council contract labels Oct 2, 2023
@alecslupu alecslupu added this to the 0.28.0 milestone Oct 2, 2023
@alecslupu alecslupu marked this pull request as ready for review October 2, 2023 09:47
@alecslupu alecslupu requested a review from a team October 2, 2023 09:47
andreslucena
andreslucena requested changes Oct 2, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

I think the problem is with the layout itself. For what I've seen in others modules we don't have this problem (i.e. with Conferences' Diplomas/Certificates of Attendance).

I'd prefer to just drop this layout altogether:

git rm ./decidim-assemblies/app/views/layouts/decidim/admin/assembly_members.html.erb

And drop the reference at

decidim/decidim-assemblies/app/controllers/decidim/assemblies/admin/assembly_members_controller.rb

Line 10 in c262495

layout "decidim/admin/assembly_members"

Then the layout is inferred using the participatory_space_layout at Concerns::AssemblyAdmin, so everything works as expected.

Can you apply those changes 🙏🏽 ?

@andreslucena andreslucena changed the title Redesign: Add assembly members sidebar menu Redesign: fix assembly members sidebar menu Oct 2, 2023
@andreslucena andreslucena added type: fix PRs that implement a fix for a bug no-backport Pull Requests that should not be backported labels Oct 2, 2023
@alecslupu alecslupu requested a review from andreslucena October 2, 2023 11:28
andreslucena
andreslucena previously approved these changes Oct 2, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

👍🏽

andreslucena
andreslucena previously approved these changes Oct 2, 2023
View reviewed changes
andreslucena
andreslucena previously approved these changes Oct 2, 2023
View reviewed changes
@alecslupu alecslupu force-pushed the fix/assembly-members-menu branch from c4cea04 to db00edf Compare October 2, 2023 15:25
@alecslupu alecslupu requested a review from andreslucena October 2, 2023 16:02
This was referenced Oct 2, 2023
Closed
Merged
@andreslucena
Copy link
Copy Markdown
Member

andreslucena commented Oct 4, 2023

@alecslupu please don't --force push after a review. It makes more difficult the reviews. I see that I've approved it after the changes proposed by me in a past review, but seeing the code it seems like my solution was actually wrong and you have to re do it. Is that correct?

@alecslupu
Copy link
Copy Markdown
Contributor Author

alecslupu commented Oct 4, 2023

@alecslupu please don't --force push after a review. It makes more difficult the reviews. I see that I've approved it after the changes proposed by me in a past review, but seeing the code it seems like my solution was actually wrong and you have to re do it. Is that correct?

The solution proposed was actually removing a template, which made few tests to fail. I tried to revert the change as there was a button on sidebar, then I have moved the button near the title.

@alecslupu
Copy link
Copy Markdown
Contributor Author

alecslupu commented Oct 4, 2023

Before:
image

After:
image

@alecslupu
Copy link
Copy Markdown
Contributor Author

alecslupu commented Oct 4, 2023

The change is actually makes it consistent with "Assembly Admins"
image

@andreslucena
Copy link
Copy Markdown
Member

andreslucena commented Oct 4, 2023

Makes perfectly sense!! I didn't take into account the "New assembly member" button thing.

andreslucena
andreslucena approved these changes Oct 4, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

👍🏽

@andreslucena andreslucena merged commit b284dda into develop Oct 4, 2023
@andreslucena andreslucena deleted the fix/assembly-members-menu branch October 4, 2023 08:24
entantoencuanto added a commit that referenced this pull request Oct 6, 2023
@entantoencuanto
Merge branch 'feature/renaming-redesign' into feature/redesign-staging
ab520b6 
* feature/renaming-redesign:
  Use default current participatory space scope as root on scopes_select_tag called from bulk actions
  restore the imports thing (compilation fails for initiatives)
  change testing color: dequelabs/axe-core#3513 (comment)
  Fix admin redesign module (#11648)
  Apply flash styles to Announcements (#11708)
  Refactor oneliners of redesigned_a11y.js (#11713)
  Redesign: fix votings admin module issues (#11704)
  Add meta robots noindex to search and profile (#10120)
  Refactor dropdown scroll to menu (#11710)
  Remove unused partial
  Remove REDESIGN_PENDING obsolete comand
  Redesign: fix assembly members sidebar menu (#11699)
  Redesign: fix conferences admin module issues (#11703)
  Redesign: fix assemblies admin module issues (#11702)
@alecslupu alecslupu mentioned this pull request Oct 14, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andreslucena andreslucena andreslucena approved these changes

Assignees

No one assigned

Labels

module: assemblies no-backport Pull Requests that should not be backported project: redesign Barcelona City Council contract type: fix PRs that implement a fix for a bug

Projects

No open projects
Maintainers' backlog
Archived in project

Milestone

0.28.0

Development

Successfully merging this pull request may close these issues.

2 participants

@alecslupu @andreslucena