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

Upgrade React Mega Menu Application Customizer Extension to latest version of SPFx - Refresh Rangers Hacktoberfest Event #1440

Open
Adam-it opened this issue Oct 21, 2024 · 15 comments · May be fixed by #1459
Assignees
Labels
Good First Issue 🏆 hacktoberfest sample: react-mega-menu status:working-on-it Known issue / feature being addressed. Will use other "status:*" labels & comments for more detail.

Comments

@Adam-it
Copy link
Member

Adam-it commented Oct 21, 2024

🎯Aim

The aim of this issue is to upgrade the React Mega Menu Application Customizer Extension extension sample to the latest version of SPFx (1.20)

🤔 Tips how to get started

To support you in this issue you may use the below tooling:

📣 What's this event all about

Refresh Rangers is a unique Hacktoberfest event aiming to update our most popular samples 🚀.
During this, you may obtain a unique Microsoft 365 & Power Platform Community badge - Refresh Rangers 🎖️
To obtain this badge besides opening a PR you also need to opt-in the Microsoft 365 & Power Platform Community Recognition Program and fill out this form

@ervingayle
Copy link
Contributor

Hi @Adam-it,
I would also like to take this one.

@Adam-it Adam-it added status:working-on-it Known issue / feature being addressed. Will use other "status:*" labels & comments for more detail. and removed help wanted labels Oct 22, 2024
@Adam-it
Copy link
Member Author

Adam-it commented Oct 22, 2024

Hi @Adam-it,
I would also like to take this one.

All Yours 👍
Please do double check if the project still works after upgrading to latest version of SPFx.
To get a badge from Microsoft 365 & Power Platform community you need to join the recognition program: https://pnp.github.io/recognitionprogram/
You only need to do that one time so if you done already in the past you don't need to fill the form again.

To get a badge from the Hacktoberfest organizers you need to register to Hacktoberfest event: https://hacktoberfest.com/participation/

You Rock 🤩👏👏👏

@Adam-it
Copy link
Member Author

Adam-it commented Oct 29, 2024

@ervingayle what is the status on this one?
The event is ending with October and this will mean that this won't count toward the Hacktoberfest event if you participated in this one (here there is nothing we may do about it)
BUT a PR for this issue will be created even after October this will still count to get the Refresh Ranger badge from the Microsoft 365 and Power Platform Community

@ervingayle
Copy link
Contributor

Hi @Adam-it
I am working on this and hoping to submit the PR tomorrow EST time. There are a few issues that I am working through and also the test functionality. It was my hope to finish this today and potentially get one more task if someone wanted to reassign it to have it done by the end of October.

@Adam-it
Copy link
Member Author

Adam-it commented Oct 29, 2024

Hi @Adam-it
I am working on this and hoping to submit the PR tomorrow EST time. There are a few issues that I am working through and also the test functionality. It was my hope to finish this today and potentially get one more task if someone wanted to reassign it to have it done by the end of October.

All good and no rush. Just wanted to align with you where we are 👍. Awesome work 👏

Happy Coding 🤩

@Adam-it
Copy link
Member Author

Adam-it commented Dec 22, 2024

hey @ervingayle there is no pressure or priority on this issue but I was wondering if you have any updates on this case?

@ervingayle
Copy link
Contributor

Hey @Adam-it I have an updated solution to SPFx version 1.20. The issue that has delayed me is migrating all of the tests from Enzyme with or without the React 17 adapter. The third party adapter supports React 17.0.2 and this causes a warning for SPFx packages. I tried moving to the React Testing Library. The mocks for office-ui-fabric-react and sp-core-library are working. What remains is just determining if it is worthwhile to migrate the tests that verify state (which react testing library does not support) or not. Perhaps I can remove the tests for the initial commit. What are your thoughts?

@Adam-it
Copy link
Member Author

Adam-it commented Dec 23, 2024

Thanks for the feed.
Are we talking about removing all of the tests or just some couple of them that test the state?

@ervingayle
Copy link
Contributor

@Adam-it Some tests are also not working which count the number of items that have been added to the menu in addition to what I mentioned about the state. This is specific to the tests and not the actual implementation itself.

@Adam-it
Copy link
Member Author

Adam-it commented Dec 25, 2024

@Adam-it Some tests are also not working which count the number of items that have been added to the menu in addition to what I mentioned about the state. This is specific to the tests and not the actual implementation itself.

But do they fail because now we have a different number of items or they just don't work due to state management?

@ervingayle
Copy link
Contributor

Hi @Adam-it . Sorry if it is not clear. There are state tests failing and after component mount tests that check the number of items that are present that are failing. However, there is a test that I added to check the presence of the data source as a before component did mount action and that shows the correct number of items. However, it doesn't work in the second set of tests.

@Adam-it
Copy link
Member Author

Adam-it commented Dec 25, 2024

Hi @Adam-it . Sorry if it is not clear. There are state tests failing and after component mount tests that check the number of items that are present that are failing. However, there is a test that I added to check the presence of the data source as a before component did mount action and that shows the correct number of items. However, it doesn't work in the second set of tests.

I get it. If we double check and confirm all is working fine and it is just a problem with tests due to a lack of support in the react testing library then I think we may remove them and skip the additional effort of migrating them.

@ervingayle
Copy link
Contributor

Hi @Adam-it. I figured out what was happening with the tests (state, screen and elements) and its working. PR will be created shortly

@ervingayle ervingayle linked a pull request Dec 31, 2024 that will close this issue
@Adam-it
Copy link
Member Author

Adam-it commented Dec 31, 2024

Hi @Adam-it. I figured out what was happening with the tests (state, screen and elements) and its working. PR will be created shortly

Awesome news!
Thank you for opening the PR. I will try to review it ASAP.
BTW Happy New Year!

@ervingayle
Copy link
Contributor

Hi @Adam-it, Happy New Year to you as well. Thank you for your contributions, guidance and help through all of this. I made a slight update to my PR to disable the debug setting for the mega menu. Just an FYI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue 🏆 hacktoberfest sample: react-mega-menu status:working-on-it Known issue / feature being addressed. Will use other "status:*" labels & comments for more detail.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants