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

[IMPROVE] refactor hard-coded component data into parameters (react component props) #77

Closed
wants to merge 78 commits into from

Conversation

nishant23122000
Copy link
Contributor

Proposed changes

Put all Hardcoded content outside of component in app/data folder and improved the responsiveness of Infotiles and carousel component by adding media query.

ScreenShot

Before

Screenshot from 2022-02-10 15-00-57

Screenshot from 2022-02-10 15-00-20

After

Screenshot from 2022-02-10 15-00-27

Screenshot from 2022-02-10 15-01-03

Copy link
Member

@Sing-Li Sing-Li left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nishant23122000 this is REALLY GOOD attempt! Glad you can "see" where we're going with this!! 👏

Please take up the challenge of:

Instead of factoring the massive parameter data seed structures as external-to-components data file (which is a paradox because they can and only be used by THAT INSTANCE of THAT COMPONENT) into global code space and polluting it .........

... add enough code into the components to make ALL OF THE DATA incoming props

This means that the data will ONLY be used to parameterize INSTANCES of the component(s) via props

<Component   data1="...some part of the data..."  data2="_more part of the data...">

within something like the index.js page or one of the demo pages pages/[_ids].js .

Hope you get what I'm talking about.

@Sing-Li Sing-Li changed the title [IMPROVE] Rafactor And Responsive [IMPROVE] refactor hard-coded component data into parameters (react component props) Feb 12, 2022
@nishant23122000
Copy link
Contributor Author

nishant23122000 commented Feb 16, 2022

Hi @Sing-Li , I think you are trying to explain that we should fetch hard-coded data from strapi and then pass it to any component as a prop as we did in the index.js file.

Let's say we have infotiles, we will create a collection for tiles in strapi and then fetch data from there. Right?

Will fetch array and then use the map in component as i did.

@Sing-Li
Copy link
Member

Sing-Li commented Feb 16, 2022

@nishant23122000 For infotiles - partially yes. But let say for Github or Leaderboard components - we want parameterization at the "backend level" So the argument would be a URL for a Github repository, or URL to an instance of a contributors leaderboard.

For one single infotile - I think the parameterization should be right on the props of the component - say with Title , Description, TileImage

How we should initialize a collection of infotiles with data from strapi -- needs to be thought out some more. The mechanism should NOT hinder the use of one single infotile on a page (in which case it doesn't make sense to have the data always in strapi)

@lgtm-com
Copy link

lgtm-com bot commented Feb 17, 2022

This pull request introduces 1 alert and fixes 1 when merging ca58c55 into 3927e64 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

fixed alerts:

  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Feb 17, 2022

This pull request fixes 1 alert when merging d0bae71 into 3927e64 - view on LGTM.com

fixed alerts:

  • 1 for Unused variable, import, function or class

sidmohanty11 and others added 20 commits February 21, 2022 00:43
…Chat#55)

Signed-off-by: Debdut Chakraborty <debdut.chakraborty@rocket.chat>
Towards a more componentized structure.
* first model (static)

* circles svg

* using strapi for fetching speakers and speaker cards using infotiles

* clean up

* using default layout and some design tweaks
Add initial support docs
Allowing presenter to chat with participants
Route all messages through the tested and proven JS SDK, initial cut
Includes a form designer tool that can be used to render customized form components.
Also making this open source IDM solution our default.
…in Forms (RocketChat#59)

* Fix Typos in JSX and add keys to props

* Fix Merge Conflicts
Fix problem with re-rendering causing major lag.
Tested.    Will be using this for the Alumni Summit
debdutdeb and others added 26 commits April 3, 2022 03:37
…Page (RocketChat#126)

* Add intro, networking and final notes slot
fix some infos from speakers

* Remove duplicate bio

* Remove duplicate talk_summary

* Remove duplicate bio

Co-authored-by: Debdut Chakraborty <debdut.chakraborty@rocket.chat>
)

* [IMPROVE] Add Rudrank as speaker

* - Add Rudrank as Speaker
- Remove unecessary data from non talk events

* [FIX] Remove other unecessary data from WELCOME

* [FIX] Rudrank gsoc previous participation
* Restyle mainstage

* Add error alert

* remove unused code

* Update app/components/clientsideonly/videostreamer.js

Co-authored-by: Debdut Chakraborty <debdut.chakraborty@rocket.chat>
* improve: realtime emoji animation

* working with prod socket conn and removing env deps

Co-authored-by: Debdut Chakraborty <debdut.chakraborty@rocket.chat>
* Add geoip support

* move readme a folder up

* Resolve conflicts and improve alert
* Call function client side

* remove console
* Add link

* Correct link
* Add link

* Correct link

* Fix clock
* navbar_ui_changes

* check_1

* check_2
@CLAassistant
Copy link

CLAassistant commented Apr 14, 2022

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
11 out of 12 committers have signed the CLA.

✅ sidmohanty11
✅ Sing-Li
✅ debdutdeb
✅ samad-yar-khan
✅ nishant23122000
✅ demonicirfan
✅ abhinavkrin
✅ dudanogueira
✅ RonLek
✅ Dnouv
✅ nik23122000
❌ Rohan Lekhwani


Rohan Lekhwani seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@lgtm-com
Copy link

lgtm-com bot commented Apr 14, 2022

This pull request introduces 1 alert and fixes 1 when merging 21704ff into 6dbe41c - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

fixed alerts:

  • 1 for Unused variable, import, function or class

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet