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

Feat: Add social icons #48

Merged
merged 22 commits into from
Oct 16, 2021
Merged

Conversation

dubisdev
Copy link
Contributor

@dubisdev dubisdev commented Oct 11, 2021

Guidelines

This pull request regarding on

  • fixed an issue
  • developed new feature
  • fixed a bug

Briefly describe what you did

I am thinking of definitely migrating from Linktree but I am missing a very important component: the social media icons.

image

At the moment I have only developed the preview using the pagelinks info, I have not touched anything in the database. I would like to know your opinion on how I should structure the data: should I create a new separate table? In my opinion it would be the best option because the icons do not have text like the other links, but I would like to know your opinion.

I was also thinking about where to create the layout for editing the links. My idea is to create another form in the formwrapper next to the links tab, called "Social". Does it seem correct?

Lastly, I would like to know which linter setting to use so as not to screw anything up :)

Appreciate your contribution to making Linkin better 🚀.

@vercel
Copy link

vercel bot commented Oct 11, 2021

Someone is attempting to deploy a commit to a Personal Account owned by @RizkyRajitha on Vercel.

@RizkyRajitha first needs to authorize it.

@dubisdev dubisdev changed the title Feat: Add media icons Feat: Add social icons Oct 11, 2021
@RizkyRajitha
Copy link
Owner

@dubisdev impressive work 🔥 .
i was also thinking about adding this feature .

should I create a new separate table?

this is a good option , you can go ahead with this. add the basic columns for now

  iconClass   String?   @db.VarChar
  linkUrl     String?   @db.VarChar
  bgColor     String?   @db.VarChar
  borderRadius     String?   @db.VarChar
  accentColor String?   @db.VarChar
  active      Boolean?  @default(true)
  created_at  DateTime? @default(now()) @db.Timestamptz(6)
  pagedata    pagedata? @relation(fields: [pagedataid], references: [id])

we can adjust them later

My idea is to create another form in the formwrapper next to the links tab, called "Social". Does it seem correct?

that sound about right 👌.
but new form you create should be like the links form , meaning we should be able to dynamically add and delete social icons , so i suggest to look at that , start by normally updating (not real time) and then we can improve it little by little.

// ignore this

watch((data, { type }) => {

// ignore this
const debouncedSaveLinkData = useCallback(

// add a save button to the form and call update directly from that submit handler.

updateLink(data);

that will ease your workflow.

Lastly, I would like to know which linter setting to use so as not to screw anything up :)

i am currently using prettier with default settings , i think that will work for now , i am hoping to add editor config with the next release .

this sounds like a lot of work so take your time and work on this

thanks

Comment on lines +11 to +24
id Int @id @default(autoincrement())
pagedataid Int?
iconClass String? @db.VarChar
displayText String? @db.VarChar
linkUrl String? @db.VarChar
bgColor String? @db.VarChar
borderRadius String? @db.VarChar
textColor String? @db.VarChar
accentColor String? @db.VarChar
active Boolean? @default(true)
created_at DateTime? @default(now()) @db.Timestamptz(6)
pagedata pagedata? @relation(fields: [pagedataid], references: [id])
orderIndex Int @default(autoincrement())
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

nothing changes just linter

@dubisdev
Copy link
Contributor Author

dubisdev commented Oct 12, 2021

To-dos:

  • DB table
    • Add prisma model
    • Create migration
    • /api endpoints
    • endpoint methods from lib/dbfuncprism.js
    • Update Readme API
  • Formwrapper
    • Create Social form layout (just JSX)
    • Add social form to Formwrapper menu
    • Connect form to db (with submit handler)
    • Connect form to db (automatic)

@RizkyRajitha
Copy link
Owner

hi @dubisdev ,

it's ok if you start with basic form (without drag and reordering ) , and after successfully implementing that you can move to drag and reordering , do as you wish .

@dubisdev
Copy link
Contributor Author

Hey @RizkyRajitha

I think this is almost done. We should try to migrate from an old database to the current one, but in principle there should be no problems :)

image

image

There are a lot of new features that could be added:

  • Choose whether to put it in or below the links
  • Remove the background (leave only the icons)
  • Select the color of the icons (here we have not added the text color property 👇)
  iconClass   String?   @db.VarChar
  linkUrl     String?   @db.VarChar
  bgColor     String?   @db.VarChar
  borderRadius     String?   @db.VarChar
  accentColor String?   @db.VarChar
  active      Boolean?  @default(true)
  created_at  DateTime? @default(now()) @db.Timestamptz(6)
  pagedata    pagedata? @relation(fields: [pagedataid], references: [id])

When you can, try it and tell me what you think :)

@RizkyRajitha
Copy link
Owner

@dubisdev great work 🔥 .

give me sometime to review this changes with an test with existing database and after that i will merge this pr.
after that we can add the other features you mentioned like icon color etc.

thanks for your contribution .

pages/_app.js Outdated Show resolved Hide resolved
Copy link
Owner

@RizkyRajitha RizkyRajitha left a comment

Choose a reason for hiding this comment

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

i have added some comments ,

@dubisdev
Copy link
Contributor Author

I think everything is done!

It might be positive for the scalability of the project to separate the endopints by features (/api/social, / api/pages).
It would facilitate scalability and allow to share file names with the same functionalities (updatelink.js for both social and page links but in different endpoints/folders).
If it's okay with you, I'll open an issue and we can talk about it there (as it is not something that expressly belongs to social icons feature).

If it is necessary to change anything else, do not hesitate to tell me 💬

@RizkyRajitha
Copy link
Owner

hi @dubisdev

It might be positive for the scalability of the project to separate the endopints by features (/api/social, / api/pages).

yes , that would be better and less confusing , you are always welcome to open issues so it can be fixed without getting missed,

give me some time to review this pr , since it is pretty big feature i have to check and also test with different scenarios before merging , so don't worry i will merge this pr as soon as every is checked .

thanks for your valuable contribution 🔥 .

@codecov-commenter
Copy link

codecov-commenter commented Oct 16, 2021

Codecov Report

Merging #48 (78cbdec) into dev (b250208) will decrease coverage by 26.69%.
The diff coverage is 0.00%.

❗ Current head 78cbdec differs from pull request most recent head 48b8cef. Consider uploading reports for the commit 48b8cef to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##              dev      #48       +/-   ##
===========================================
- Coverage   72.52%   45.83%   -26.70%     
===========================================
  Files           3        3               
  Lines          91      144       +53     
  Branches       18       28       +10     
===========================================
  Hits           66       66               
- Misses         21       69       +48     
- Partials        4        9        +5     
Impacted Files Coverage Δ
lib/dbfuncprisma.js 44.77% <0.00%> (-29.30%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b250208...48b8cef. Read the comment docs.

@RizkyRajitha RizkyRajitha merged commit 1c3ea4e into RizkyRajitha:dev Oct 16, 2021
@dubisdev dubisdev deleted the feat-social-links branch October 17, 2021 15:07
@dubisdev dubisdev mentioned this pull request Oct 17, 2021
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants