UI and Code refactoring for release v0.0.3

[sidmohanty11]

UI and Code Refactoring for release v0.0.3

UI

These are just my suggestions, if you have any amazing UI/UX idea please feel free to suggest it

Code

• [x] Multiple API requests being made (refer #112) -> occurring at ChatBody.js while calling getMessagesAndRoles() twice
• [ ] Circular dependencies error from terminal [0] (!) Circular dependencies [0] src/components/Markup/elements/ItalicSpan.js -> src/components/Markup/elements/BoldSpan.js -> src/components/Markup/elements/ItalicSpan.js [0] src/components/Markup/elements/BoldSpan.js -> src/components/Markup/elements/StrikeSpan.js -> src/components/Markup/elements/BoldSpan.js [0] src/components/Markup/elements/ItalicSpan.js -> src/components/Markup/elements/BoldSpan.js -> src/components/Markup/elements/StrikeSpan.js -> src/components/Markup/elements/ItalicSpan.js
• [ ] src/components/auth -> unnecessary Login.css, Login.module.css has improper casing - same with LoginForm, TwoFactorTotpModal.module.css
• [x] src/components/Attachments/Attachment has reference to hard-coded value to localhost - use RCInstance.getHost()
• [ ] src/components/ChatHeader - unused variables, do we need them?
• [x] src/components/MessageList -> key is missing
• [ ] src/components/SearchMessage unused variables
• [ ] src/components/TotpModal unused import
• [ ] Refactor three useEffects inside src/index.js
• [ ] Sometimes while development its giving a huge CSS loader error, does anybody know why?
• [ ] Ensure we use ESLint throughout the codebase and not disable rules (we can disable those which are not in our hands, for e.g, _id - dangling underscore) to get benefit of linting

[VipinDevelops]

Hey @sidmohanty11 could you please assign me this issue I noticed Multiple API requests being made ill look into that and fix that thing and the circular dependencies error mentioned in the terminal? right Additionally, I noticed some other potential issues in the codebase such as unnecessary files, hard-coded values, unused variables and imports, and missing keys in the message list file. I would be happy to fix those as well. Lastly yes we can make sure to use ESLint throughout the codebase and avoid disabling rules, except for those outside of our control like the dangling underscores like you mentioned ill look into it deeply. I am excited about v0.0.3 of EmbeddedChat and after these updates, Embeddedchat will be completely ready for a new release. Thank You!

[VipinDevelops]

About the Task 5, I removed all the unused variables from the Chatheader component in this PR

[sidmohanty11]

@VipinDevelops sure! Please add your issues/findings here as a task list and let's go through them from here only so that we can ready ourselves for 0.0.3 launch.

And please keep PRs separate so that it will be easier for me to review. Thanks!

[VipinDevelops]

Yeah sure ill go through the code deeply and will put new issues or any potential code convention I found Will try my best to organize the Pull request in a good manner so it will be easier for you to manage.

and yeah ill manage if any new contributors show interest in working on one of these issues and help them with that.

Thank you!

[sidmohanty11]

@VipinDevelops please keep them separate

[VipinDevelops]

@VipinDevelops please keep them separate

Sure i will keep this in mind

[VipinDevelops]

I am starting with Tasks1 as it will solve two issues as mentioned , I will check which API call is necessary and how can we store and reuse the information to avoid making other API call

anyone who is interested in working on one of these issues feels free to share your approach here

[CulturalProfessor]

@VipinDevelops I will start with Task 4 and change hardcode localhost url from /src/context/RCInstance.js gethost(). And for some UI changes I was thinking about adding a hover feature on buttons in chatInputFormattingToolbar to display info as follows: Or should i open new issue for it?

[VipinDevelops]

@VipinDevelops I will start with Task 4 and change hardcode localhost url from /src/context/RCInstance.js gethost(). And for some UI changes I was thinking about adding a hover feature on buttons in chatInputFormattingToolbar to display info as follows: Or should i open new issue for it?

Sure go ahead make sure you make different PR for each fix

And take one issue at a time

Try to build UI like what sidharth suggested in UI changes , he want that all options hidden and a mechanism to show them if user wants to try to build something like that

No need to open another issue we can fix all issues here

Thank you

[CulturalProfessor]

Ok,thanks for heads up.

@VipinDevelops I will start with Task 4 and change hardcode localhost url from /src/context/RCInstance.js gethost(). And for some UI changes I was thinking about adding a hover feature on buttons in chatInputFormattingToolbar to display info as follows: Or should i open new issue for it?

Sure go ahead make sure you make different PR for each fix

And take one issue at a time

Try to build UI like what sidharth suggested in UI changes , he want that all options hidden and a mechanism to show them if user wants to try to build something like that

No need to open another issue we can fix all issues here

Thank you

[CulturalProfessor]

@VipinDevelops I will start with Task 4 and change hardcode localhost url from /src/context/RCInstance.js gethost(). And for some UI changes I was thinking about adding a hover feature on buttons in chatInputFormattingToolbar to display info as follows: Or should i open new issue for it?

@VipinDevelops hey regarding task 4: I changed url from localhost:3000 to host prop.

[VipinDevelops]

I changed url from localhost:3000 to host prop.

We have a function this "gethost" you can find that in lib/api file use that function

[CulturalProfessor]

We have a function this "gethost" you can find that in lib/api file use that function

@VipinDevelops I have used RCInstance getHost().

[VipinDevelops]

We have a function this "gethost" you can find that in lib/api file use that function

@VipinDevelops I have used RCInstance getHost().

No not like this don't make a new instance take a look at this and implement something similar

[CulturalProfessor]

No not like this don't make a new instance take a look at this and implement something similar

Ok now I think it is okay?

[VipinDevelops]

Yeah looks good to me you can make sure its working by console log the host and i think you are good to make a PR

[KoushikAD1234]

Hey @VipinDevelops I will start work on this avatar spacing.

And this overflow one.

Do I need to raise an issue?

[VipinDevelops]

No you don't need to create another issue we will fix all these issue by making seperate PR's make sure you create different PR's for both issue

[CulturalProfessor]

@VipinDevelops @sidmohanty11 I have made a PR for task 4.https://github.com/RocketChat/EmbeddedChat/pull/133

[CulturalProfessor]

@VipinDevelops regarding task-6 is MessageReactions component missing key in map ?

[imf-ali]

@VipinDevelops regarding task-6 is MessageReactions component missing key in map ?

Just move the key attribute of Message component into Box component

[CulturalProfessor]

Just move the key attribute of Message component into Box component

Ok right?

[imf-ali]

Yup, this should fix the key missing props

[KoushikAD1234]

Hey @VipinDevelops I have just made my first PR by fixing the weird avatar spacing and also changes the squared shape avatar box to rounded. Please Review once.

[VipinDevelops]

Hey @VipinDevelops I have just made my first PR by fixing the weird avatar spacing and also changes the squared shape avatar box to rounded. Please Review once.

Changes look fine to me can you add a screenshot to your PR to show the changes you made Thank you

[CulturalProfessor]

Hey @sidmohanty11 regarding ui change of ChatInputFormattingToolbar can I replace it with a dropdown menu with options for formatting similar to menu in chat header to the left of ChatInput component?

[honey-grapes]

Hi, has anyone started with the Task 7 or 8 yet? (src/components/SearchMessage unused variables, src/components/TotpModal unused import). If not maybe I can take a stab.

[CulturalProfessor]

Hi, has anyone started with the Task 7 or 8 yet? (src/components/SearchMessage unused variables, src/components/TotpModal unused import). If not maybe I can take a stab.

I don't think anyone is on it.

[honey-grapes]

Hi, has anyone started with the Task 7 or 8 yet? (src/components/SearchMessage unused variables, src/components/TotpModal unused import). If not maybe I can take a stab.

I don't think anyone is on it.

Ok cool, let me try working on it!

[umangutkarsh]

Hi @sidmohanty11 , I've created a PR, please review it and let me know if any changes are required. Hope you'll consider this. Thanks

[umangutkarsh]

@sidmohanty11 , what all tasks are pending in this issue? Can you confirm?

[WiseyXD]

@sidmohanty11 @VipinDevelops , what all tasks are pending in this issue? Can you confirm?

[arpitaneware06]

@sidmohanty11 can you please give little bit description .

[sidmohanty11]

After GSoC, @abhinavkrin has made a lot of changes so I don't think this is still relevant. Closing this for now, if you feel like there are some minor issues remaining feel free to create an issue for it