freeCodeCamp

freeCodeCamp copied to clipboard

Checklist:

• [x] I have read freeCodeCamp's contribution guidelines.
• [x] My pull request has a descriptive title (not a vague title like Update index.md)
• [x] My pull request targets the main branch of freeCodeCamp.
• [x] I have tested these changes either locally on my machine, or GitPod.

Closes #47356

:eyes: Review this PR in a CodeSee Review Map

View the CodeSee Map of this change

Legend

@shootermv, Thank you for this pull request. Not sure if this pr is a work in progress, but I wanted to point a few things that are missing. The main aim of the component library is to replace the react-bootstrap library with minimum to no modifications on the lean's client side.

So our components should be identical to the used react-bootstrap ones:

• they should receive the same props
• have a similar logic
• look the same
• output the same elements

To clarify the technical specifications, it would be great if you could research the current usage as described in the documentation and create an issue. Then, you could finalize this pr accordingly.

What @ahmadabdolsaheb said, but I would also add that we should try to keep the component library "dumb", in that they don't know about the business logic of the consumers / the apps that use them. It is so that they can be shared across multiple apps (/learn is the main consumer, but we are aiming to use the library in Code Radio and Chapter as well). This means the drop down shouldn't have "Account Settings" or "Sign out" options built in.

Let us know if you need our help with the development process 🙂

can i see some place that project currently has this dropdownButton functionality?

@shootermv, here are multiple use cases: https://github.com/freeCodeCamp/freeCodeCamp/search?q=DropdownButton Doing a search for DropdownButton on your editor will give you better results.

@shootermv, the component is taking shape pretty nicely. Let me know if I could be of any help or if you need any further clarification.

currently i added buttonText and options props, dont sure what to do next

@shootermv so the implementation in this pr has different styles and functionality comparing to the bootstrap component in the codebase. I highly recommend reviewing the https://github.com/freeCodeCamp/freeCodeCamp/issues/44668 issue, the related documentation (mentioned in the same issue) and create an issue accordingly so we could finalize the specifications before proceeding with updating this pr.

@shootermv how is the progress going on your side. Let me know if you need any help.

Hey @shootermv.

Are you still planning to work on this? If not, we can open it to contributors.

Have a nice day and happy coding

@shootermv, the aim is to just replace the source of the components and have them work. Here are a few considerations:

• [ ] The options prop is not needed. Each Menu.Item needs to be a stand alone component ( MenuItem )
• [ ] MenuItem does not need to stay in a separate directory and can be a part of the Drop-Down-Button story.
• [ ] The applied animation is beautifully done, but animation is not needed at this point. We will unify and reimplement all animation on the next phases.
• bottonText prop should be called Title
• [ ] Since we are using headless UI we have less control over the output of the components, so it is ok if they don’t share the same structure with the react-bootstrap ones.
• [ ] The dropdown used on the mobile version of legacy certifications (tool-panel.tsx) is actually a drop-up. Feel free to pass a direction prop (‘up’) to the currently used DropDownButton component in the Editor, and clean up the styles there. Once that is done, have the recently developed component accept that prop and default to ‘down’ if direction prop is not passed.
• [ ] Currently 5 props are being passed to the old DropDownButton component. Since block, ‘bsStyle’, and className are being passed the similar values, you don’t need a logic to differentiate if those props are being passed or not. Just make sure Typescript does not error out.
• [ ] Free free to expand on the tests by looking at other component tests in our ui library and by searching for dropdown in the react-bootstrap repo and incorporate the relevant ones in our library.

Once those changes are in, I will give it a final round of review and proceed with the merge.

Let me know if you have other questions.

only now I figured the problem that can be here: freecodecamp uses bootstrap.css styles but my DropDownButton component - uses headless (which uses tailwind.css) to import both of css framework simultaneously - can be problematic...

The currently used components use boostrap styles and some custom styles. The newly developed components (such as this one) do not need bootstrap or custom styles to work. They should replicated the current look only using tailwind classes.

You can look into similarly built components on the ui-components directory.

@shootermv, the aim is to just replace the source of the components and have them work. Here are a few considerations:

• The options prop is not needed. Each Menu.Item needs to be a stand alone component ( MenuItem )
• MenuItem does not need to stay in a separate directory and can be a part of the Drop-Down-Button story.
• The applied animation is beautifully done, but animation is not needed at this point. We will unify and reimplement all animation on the next phases.
• bottonText prop should be called Title
• Since we are using headless UI we have less control over the output of the components, so it is ok if they don’t share the same structure with the react-bootstrap ones.
• The dropdown used on the mobile version of legacy certifications (tool-panel.tsx) is actually a drop-up. Feel free to pass a direction prop (‘up’) to the currently used DropDownButton component in the Editor, and clean up the styles there. Once that is done, have the recently developed component accept that prop and default to ‘down’ if direction prop is not passed.
• Currently 5 props are being passed to the old DropDownButton component. Since block, ‘bsStyle’, and className are being passed the similar values, you don’t need a logic to differentiate if those props are being passed or not. Just make sure Typescript does not error out.
• Free free to expand on the tests by looking at other component tests in our ui library and by searching for dropdown in the react-bootstrap repo and incorporate the relevant ones in our library.

Once those changes are in, I will give it a final round of review and proceed with the merge.

Let me know if you have other questions.

can you please change this list to checkboxes one, and i will mark already accomplished points? Thanks

must understand how correctly import this tools/ui-components project to client in order to test dropdownbutton there

must understand how correctly import this tools/ui-components project to client in order to test dropdownbutton there

You need to npm run build and npm run build:css from the ui-components directory. Once the builds are finished, import the bundle file and the css file to your desired learn component.

The path should be something similar to the following. (not sure about the number of ../ s :) import { MenuItem } from '../../../../../tools/ui-components/dist/bundle.esm'; '../../../../../tools/ui-components/dist/base.css';

If you are getting errors with scss, that is expected. You would add an empty postcss.config.js file in the root repository and things should be smooth.

Let me know if you have further questions.

Actually here is one of questions im currently thinking about: How we plan new coponents to be used at the site? Will it be simultaneoulsy with bootstrap styles?(which site currently has) Thanks

On Tue, Nov 15, 2022, 16:31 Ahmad Abdolsaheb @.***> wrote:

must understand how correctly import this tools/ui-components project to client in order to test dropdownbutton there

You need to npm run build and npm run build:css from the ui-components directory. Once the builds are finished, import the bundle file and the css file to your desired learn component.

The path should be something similar to the following. (not sure about the number of ../ s :) import { MenuItem } from '../../../../../tools/ui-components/dist/bundle.esm'; '../../../../../tools/ui-components/dist/base.css';

If you are getting errors with scss, that is expected. You would add an empty postcss.config.js file in the root repository and things should be smooth.

Let me know if you have further questions.

— Reply to this email directly, view it on GitHub https://github.com/freeCodeCamp/freeCodeCamp/pull/47043#issuecomment-1315393473, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKGMHREW2YYUJAAF4GDFIDWIONDDANCNFSM54V6ZCWA . You are receiving this because you were mentioned.Message ID: @.***>

No they should be used regardless. We should rely on bootstrap classes or styles. The following two conditions must be met:

1. new component looks like the bootstrap one when imported to learn.
2. new component looks like the bootstrap one in storybook.

We are officially on Node.js 18 now. We have changed the merge requirements accordingly.

You will need to rebase this PR against the main branch OR Click the "Update branch" button to get the new checks going.

Thanks for your contribution.

Thanks for your pull-request.

We are no longer accepting changes to the non-English versions of files in parts of this codebase. This pull-request seems to change some of those. Please visit our contributing guidelines to learn more about translating freeCodeCamp's resources.

As always, we value all of your contributions.

Happy contributing!

---

Note: This message was automatically generated by a bot. If you feel this message is in error or would like help resolving it, feel free to reach us in our contributor chat.

Thanks for the hard work here. Here are a few nitpicks before we merge this pr:

• Replace up with dropup so it is consistent with the bootstrap convention.
• Please check the list for the props currently passed to the MenuItem and DropDownButton in learn, rel seems missing form the new MenuItem.
• Remove the <br/> from the story and adjust the height of the container div to account for the dropdown/dropup
• Since the MenuItem is also added to the component, please include the related story
• Include applicable tests for both components. see examples: https://github.com/react-bootstrap/react-bootstrap/tree/master/test
• Since the related issue does not include the currently used CSS, please double check that the styles used in the new component corresponds with the old ones.

not sure what do you mean here rel seems missing form the new MenuItem I looked at react-bootstrap docs and I cannot see rel property anywhere. will be glad for explanation thanks

@shootermv, I couldn't create a story even when with importing a parent.

For now, remove the extra CSS properties, and when everything is sorted we can deal with story

Since the related issue does not include the currently used CSS, please double check that the styles used in the new component corresponds with the old ones.

Thanks for your pull-request.

We are no longer accepting changes to the non-English versions of files in parts of this codebase. This pull-request seems to change some of those. Please visit our contributing guidelines to learn more about translating freeCodeCamp's resources.

As always, we value all of your contributions.

Happy contributing!

---

Note: This message was automatically generated by a bot. If you feel this message is in error or would like help resolving it, feel free to reach us in our contributor chat.

No dependency changes detected. Learn more about Socket for GitHub ↗︎

---

👍 No dependency changes detected in pull request

Pull request alert summary

Issue	Status
Install scripts	✅ 0 issues
Native code	✅ 0 issues
Bin script shell injection	✅ 0 issues
Unresolved require	✅ 0 issues
Invalid package.json	✅ 0 issues
HTTP dependency	✅ 0 issues
Git dependency	✅ 0 issues
Potential typo squat	✅ 0 issues
Known Malware	✅ 0 issues
Telemetry	✅ 0 issues
Protestware/Troll package	✅ 0 issues

Thanks for your pull-request.

We are no longer accepting changes to the non-English versions of files in parts of this codebase. This pull-request seems to change some of those. Please visit our contributing guidelines to learn more about translating freeCodeCamp's resources.

As always, we value all of your contributions.

Happy contributing!

---

Note: This message was automatically generated by a bot. If you feel this message is in error or would like help resolving it, feel free to reach us in our contributor chat.

Thanks for your pull-request.

We are no longer accepting changes to the non-English versions of files in parts of this codebase. This pull-request seems to change some of those. Please visit our contributing guidelines to learn more about translating freeCodeCamp's resources.

As always, we value all of your contributions.

Happy contributing!

---

Note: This message was automatically generated by a bot. If you feel this message is in error or would like help resolving it, feel free to reach us in our contributor chat.

I just noticed, that I have messed up and lost the Co-author in the commit history, while trying to fix the conflict.

I will squash all my commits when I am done and add the deserved attribution, when I am done 👍