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

Fixed missing/wrong admin _setActiveMenu() #4209

Merged
merged 2 commits into from
Sep 19, 2024

Conversation

sreichel
Copy link
Contributor

Description (*)

Some sections are not correctly highlighted when active.

@github-actions github-actions bot added Component: PayPal Relates to Mage_Paypal Component: Adminhtml Relates to Mage_Adminhtml Component: CurrencySymbol Relates to Mage_CurrencySymbol Component: Widget Relates to Mage_Widget Component: Oauth Relates to Mage_Oauth Component: Index Relates to Mage_Index Component: Api2 Relates to Mage_Api2 Component: ImportExport Relates to Mage_ImportExport labels Sep 19, 2024
@fballiano
Copy link
Contributor

when you take PRs from other repos you should at least mention them: MahoCommerce/maho#12

@sreichel fo you remember that you were accusing somebody of this same thing?

@sreichel
Copy link
Contributor Author

sreichel commented Sep 19, 2024

Dear @fballiano,

these changes are based on this issue #4207 and aren't identical to yours.

On your repo: +131 −110 lines changed
Here: +49 −40

Btw ... i can see a lot of "OM PR xyz" at https://github.com/MahoCommerce/maho/commits/main/.

Have you left any credits?

For sure i'll leave credits when I should pick up your or anybody else work.


Edit 22/09:

You have removed the complete history, so it looks like all work was done by you? (https://github.com/MahoCommerce/maho/graphs/contributors)

@addison74 addison74 merged commit 3d193ed into OpenMage:main Sep 19, 2024
17 checks passed
@sreichel sreichel deleted the fix-active-menu branch September 19, 2024 06:37
@OpenMage OpenMage locked as too heated and limited conversation to collaborators Sep 19, 2024
@OpenMage OpenMage deleted a comment from fballiano Sep 19, 2024
@OpenMage OpenMage deleted a comment from fballiano Sep 19, 2024
@OpenMage OpenMage deleted a comment from fballiano Sep 19, 2024
@OpenMage OpenMage deleted a comment from fballiano Sep 19, 2024
@sreichel
Copy link
Contributor Author

Removed offtopic. Backuped screenshot.

@kiatng
Copy link
Contributor

kiatng commented Sep 22, 2024

As a reviewer of this PR, I acknowledge that this PR should credit @tmewes original MahoCommerce/maho#12, which is precedent to this one. @fballiano Thanks for pointing this out.

@sreichel
Copy link
Contributor Author

Why? I wanted to fix the missing highlighting (#4207, #4208) and manually reviewed every link after i have seen that even with changed SCSS not all links were correct. Of course it looks similiar, b/c it cant be fixed in another way.

@kiatng
Copy link
Contributor

kiatng commented Sep 23, 2024

I can explain my opinion: both issues (#4207, #4208) were dated Sep 18, which is later than MahoCommerce/maho#12, dated Sep 1. Even though you may have the same idea earlier, I and others do not know about it until Sep 18. I didn't know @tmewes PR, but now that I know, I acknowledge and credit him for his idea.

[edited]
Further, you wrote
> ... manually reviewed every link ...
It seems @tmewes PR inspired this PR. If this is true, I am not an expert on ethic, but in my heart, I feel crediting the original work should be made in this case.

I am sorry for incorrectly interpreting “manually reviewed every link”.

@sreichel
Copy link
Contributor Author

sreichel commented Sep 23, 2024

I leave credits when a use others work ...

If i had picked up @tmewes PR or got inspired by it, i'd at least had left a "thanks".

No Kia, I have no need to adorn myself with borrowed plumes.

@sreichel
Copy link
Contributor Author

I am sorry for incorrectly interpreting “manually reviewed every link”.

My bad, sorry for misleading wording ... "every admin menu link".

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Component: Adminhtml Relates to Mage_Adminhtml Component: Api2 Relates to Mage_Api2 Component: CurrencySymbol Relates to Mage_CurrencySymbol Component: ImportExport Relates to Mage_ImportExport Component: Index Relates to Mage_Index Component: Oauth Relates to Mage_Oauth Component: PayPal Relates to Mage_Paypal Component: Widget Relates to Mage_Widget
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants