Skip to content

Extract dev_tools from OP monorepo module#2

Open
mrmir wants to merge 3 commits intomainfrom
add-dev-tools-gem
Open

Extract dev_tools from OP monorepo module#2
mrmir wants to merge 3 commits intomainfrom
add-dev-tools-gem

Conversation

@mrmir
Copy link
Copy Markdown

@mrmir mrmir commented Mar 4, 2026

Continuing from opf/openproject#22016

@mrmir mrmir requested review from machisuji and removed request for machisuji March 4, 2026 15:27
@mrmir mrmir requested a review from toy April 2, 2026 15:23
@mrmir
Copy link
Copy Markdown
Author

mrmir commented Apr 2, 2026

This took a while to get back to, but I incorporated whatever feedback I could from the other PR and I think it's ready to be merged now if you think so too @toy

Copy link
Copy Markdown

@toy toy left a comment

Choose a reason for hiding this comment

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

Awesome, thank you! I noted just few cleanup things and ideas


## Tools

- **User switcher**: A dropdown in the top navigation bar listing all active users. Click any user to immediately switch to them.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thought for future additions - it may be nice to be able to easily switch them on/off, like if they conflict with something or someone prefers to not have them all on

module OpenProject
module DevTools
end
end
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No need to create empty modules


register "openproject-dev_tools",
author_url: "https://www.openproject.org",
bundled: false,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Doesn't need to be explicitly specified

Suggested change
bundled: false,

#++

OpenProject::Application.routes.draw do
scope module: :dev_tools, path: "dev_tools", as: :dev_tools do
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think using namespace has same effect:

Suggested change
scope module: :dev_tools, path: "dev_tools", as: :dev_tools do
namespace :dev_tools do

classes: "op-app-menu--item",
style: "order: 1",
menu_id: "op-dev-user-switcher-menu",
select_variant: :single
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In some cases the menu renders narrower wrapping user names, so I think it is better to set size (probably to :small, as it is enough and the smallest one), this also resolves it jumping from bottom to side when user list doesn't fit vertically

end

users.each do |user|
menu .with_item(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
menu .with_item(
menu.with_item(

render(
Primer::Alpha::ActionMenu.new(
classes: "op-app-menu--item",
style: "order: 1",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I have a feeling that some people would ask you to not use inline styles if it would be in the main project, but I don't mind in dev module, up to you


current_user_name
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not sure about this, but maybe first user should be «Anonymous», as a quick way to see things logged out, also next menu has sign out


if user&.active?
login_user(user)
flash[:notice] = I18n.t("dev_tools.user_switcher.switched", name: user.name)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I assume it is not important, but logging in from logged out state doesn't show the notice

Comment on lines +6 to +9
title: "Switch User (Dev Only)"
tooltip: "Switch User (Dev Only)"
heading: "Switch User (Dev Only)"
active_users: "Active Users"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I see only tooltip used out of last 4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants