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

Reusable FAQ Component #1287

Closed
wants to merge 16 commits into from
Closed

Conversation

Monilprajapati
Copy link
Member

As per Issue #1273 , I have edited the FAQ component and improved its user interface. You can add this component anywhere, as I have raised an issue on the pricing page FAQ section, where it needs to be applied first.
@frikky Check this out!
I am currently addressing Issue #1274. Please assign it to me.

@frikky
Copy link
Member

frikky commented Dec 15, 2023

Thanks @Monilprajapati

@ayush0033 can you check if this is useful together with @iiAnna?

@Monilprajapati
Copy link
Member Author

Hey @ayush0033 !

@Monilprajapati
Copy link
Member Author

Hey @ayush0033 Please take a look and take a futher steps

@Monilprajapati
Copy link
Member Author

Hey @frikky Please take a look! and let me know what should i need to do further

@ayush0033
Copy link
Contributor

Hey @Monilprajapati, share the screenshot or screen recording of the view so we can get a better idea.

@Monilprajapati
Copy link
Member Author

Okkk @ayush0033
Here it is :

Shuffle_FAQ.mp4

@ayush0033
Copy link
Contributor

Hello @Monilprajapati, thank you for your work on the FAQ component. However, we've already implemented a similar feature on the front page. Your efforts are truly appreciated, and I'll keep your FAQ in mind for future needs. Thank you once again for your dedication and hard work.

Here is our existing FAQ component:
Screenshot 2024-04-03 at 3 44 56 PM

Shuffle.-.admin.mp4

@Monilprajapati
Copy link
Member Author

Monilprajapati commented Apr 3, 2024

Hello @Monilprajapati, thank you for your work on the FAQ component. However, we've already implemented a similar feature on the front page. Your efforts are truly appreciated, and I'll keep your FAQ in mind for future needs. Thank you once again for your dedication and hard work.

Here is our existing FAQ component: Screenshot 2024-04-03 at 3 44 56 PM

Shuffle.-.admin.mp4

My pleasure, Yeah that i know but i have raised this issue to change the FAQ section on the pricing page so you can change the UI their,
this one see :

shuffle_faq.mp4

@Monilprajapati
Copy link
Member Author

@ayush0033 One more thing I'd like to mention is that Shuffle needs to revamp its code. There's an excessive amount of code in a single file, and React isn't being utilized efficiently. The same code is repeated throughout. Please take a look into this, and if you find that it requires attention, I'm in :)

@ayush0033
Copy link
Contributor

I will definitely take a closer look at this for the pricing page to ensure its reliability.

@frikky
Copy link
Member

frikky commented Apr 4, 2024

@ayush0033 One more thing I'd like to mention is that Shuffle needs to revamp its code. There's an excessive amount of code in a single file, and React isn't being utilized efficiently. The same code is repeated throughout. Please take a look into this, and if you find that it requires attention, I'm in :)

Heyo!

Just to chime in:

  • I 100% disagree to the fact that we have "excessive amount in a single file". You shouldn't split code just to split code. You split code IF it needs to be split to be optimized rendering/processing-wise.
  • The code is indeed subpar speed-wise when it comes to states, but that's mainly in the AngularWorkflow.jsx file

What you are talking about repeated code tho - that's indeed where components shine, and we do that in most of the code written the last few years. For an FAQ section, this is overdramatization of a very simple problem where it in reality doesn't matter much.

Either way: @ayush0033 will look into it all again and we'll figure something out :)

@Monilprajapati
Copy link
Member Author

@ayush0033 One more thing I'd like to mention is that Shuffle needs to revamp its code. There's an excessive amount of code in a single file, and React isn't being utilized efficiently. The same code is repeated throughout. Please take a look into this, and if you find that it requires attention, I'm in :)

Heyo!

Just to chime in:

  • I 100% disagree to the fact that we have "excessive amount in a single file". You shouldn't split code just to split code. You split code IF it needs to be split to be optimized rendering/processing-wise.
  • The code is indeed subpar speed-wise when it comes to states, but that's mainly in the AngularWorkflow.jsx file

What you are talking about repeated code tho - that's indeed where components shine, and we do that in most of the code written the last few years. For an FAQ section, this is overdramatization of a very simple problem where it in reality doesn't matter much.

Either way: @ayush0033 will look into it all again and we'll figure something out :)

Hey @frikky

I wanted to take a moment to apologize for any unintended tone in my previous message regarding the code structure. I realized that my wording may have come across as more critical than intended 🥲 🥲

Please know that my intention was solely to offer constructive feedback aimed at improving the efficiency and organization of the codebase. I deeply value the contributions of everyone involved in the project, including yourself, and I never meant to undermine that 🥲

I appreciate your patience and understanding, and I'm fully committed to working together to find the best solutions for Shuffle. Let's continue collaborating positively to enhance the project 🙂

I will provide you with the codes that I was referring to, along with proposed solutions for each issue. I will then proceed to solve them one by one by creating individual issues. Just give me some time :)

@Monilprajapati
Copy link
Member Author

Monilprajapati commented Apr 6, 2024

So here, I have found 7 things that I was referring to in 2 days, along with their solutions. I have attached the video in which I have shown everything to get the idea quickly.

@frikky, @ayush0033

1. There is one component called OrgHeaderexpanded.jsx. We can reduce the code of that file from 873 to 530.

  • First of all, the states that we have defined in the older file are not written efficiently. We can write them in a clearer way as I have shown in the new file. By doing this, we can reduce the code by 50 lines and make it more readable.
  • The second thing is the section for editing details of the organization. The code that we had written before is of 400 lines. To reduce that code repetition, I have just created one component called HeaderInputTag.jsx and then used it in the new file. After making these changes, we can reduce that code to 100 lines.
  • So by making these changes, we can overall make the difference of 400 lines and have more readable code. 😊
  • After the changes made in the code, the output is still the same as I showed in the video.

- States in more readable format :

states.mp4

- HeaderInputTag Component Use :

HeaderInputTag.mp4

2. SettingPage in App.jsx using the same HeaderInputTag.jsx reduced tons of lines of code.

  • So as I have mentioned above, I have created one component called HeaderInputTag.jsx. By using it, we can define those fields directly without writing the same code again and again.
  • I created NewSettingsPage.jsx to check the same output after the changes. By doing this, we can achieve more code readability and it becomes easier to update.
  • If we want to change the UI of the input, we just need to make a change in one line and that change will be reflected in all of the input fields in OrgHeaderExpanded.jsx and SettingsPage.jsx.

Here is the video of that:

SettingsPage.webm

3. API call function defining multiple times in different files.

  • So there is one API call function called getAvailableWorkflows.jsx that we are using and also defining in these files which I have mentioned below:
    1. BillingStats.jsx
    2. OrgHeaderexpanded.jsx
    3. RuntimeDebugger.jsx
    4. AngularWorkflow.jsx
    5. Dashboard.jsx
    6. GettingStarted.jsx
    7. Welcome.jsx
    8. Workflow.jsx

  • Yes, so we are defining this function in all of those files which leads to code repetition.

fetch(globalUrl + "/api/v1/workflows", {
    method: "GET",
    headers: {
        "Content-Type": "application/json",
        Accept: "application/json",
    },
    credentials: "include",
})
.then((response) => {
    if (response.status !== 200) {
        console.log("Status not 200 for workflows :O!: ", response.status);
    }

    return response.json();
})

-- So what I have done is I have created one folder called services in which we can add the API calls that are being commonly used. In that folder, I have created one file called fetchWorkflow.jsx. After that, we just need to write like this to call that API:

fetchWorkflows(globalUrl)
    .then((responseJson) => {
        // Process responseJson as needed
    })
    .catch((error) => {
        // Handle errors
    });

Everything is shown in the below video:

getAvailableWorkflowAPI.webm

4. Same as getApps() API call, we are defining it multiple times.

  • So these are the files in which we are defining that call function again and again:
  1. Admin.jsx
  2. AngularWorkflow.jsx
  3. Apps.jsx
  4. RunWorkflow.jsx
  5. Welcome.jsx
  6. Workflow.jsx
  • So here, I have done the same as I have done for fetchWorkflows(). I have created one file called fetchApps.js in which I have defined the logic. After writing this, we can directly use it like this:
fetchApps(globalUrl)
    .then((responseJson) => {
        // Process responseJson as needed
    })
    .catch((error) => {
        // Handle errors
    });

Here is the video for that:

getAppsAPI.webm

5. GET method fetch API call.

  • As an example of Admin.jsx, we are using GET fetch method in so many files. So, by creating one common function, we can directly use it and save tons of lines of repetitive code.
  • I have created one file called getFetch.js in which I have defined one function called fetchData(). By the use of that, we can directly write like this in every file:
fetchData(globalUrl + "/api/v1/orgs")
    .then((responseJson) => {
        // Process responseJson as needed
    })
    .catch((error) => {
        // Handle errors
    });

Video that explains everything easily.

getFetch.webm

Also, I'd suggest considering the use of the Context API. It can help simplify state management and reduce the need for passing props through multiple layers of components. This approach not only enhances efficiency but also minimizes repetitive state setups across various files. 😊🚀

Hey @frikky I'm excited to tackle each issues that i have mentioned, create solutions, and submit PRs. Implementing the Context API sounds like a great opportunity. I'm committed to contributing effectively to Shuffle, and I'm confident in my skills. Looking forward to making a positive impact!🚀🚀

@Monilprajapati
Copy link
Member Author

Hey @frikky, please check this out. I am very excited to work on it :)

@Monilprajapati
Copy link
Member Author

Hey @frikky @ayush0033 :)

@frikky
Copy link
Member

frikky commented Apr 12, 2024

Hey @frikky @ayush0033 :)

Heyo!

Sorry, we're busy building stuff, and don't pay enough attention here. Thanks for this - it could help a lot! Are you available for a call? I think we should have a discussion about this and your contributions, as you are bringing up some good points, along with some I disagree with. Answering your message elsewhere so we can get it set up ;)

@Monilprajapati Monilprajapati closed this by deleting the head repository May 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

None yet

3 participants