Feature to share news/posts/articles by idebenone · Pull Request #162 · medyo/hackertab.dev

idebenone · 2023-09-04T17:59:35Z

Added an option to share news on the card.

medyo · 2023-09-11T10:21:03Z

@idebenone, could you please include both the title and the URL when sharing content, like this:

it would be helpful if you could show the title before sharing so that users can have some context for what they are sharing.

Thank you

idebenone · 2023-09-11T11:12:59Z

Hey @medyo !
I just did a commit with title as well as source of the article also. Please check and let me know.

medyo · 2023-09-14T22:10:21Z

@idebenone

The UI looks a bit different from other pages, Could you make it like the settings modal (with inputs and buttons), something like the rss section
Add a parent class to classes like .link .copy... to prevent potential conflicts with other components in the future
Please include the title prop for the share buttons.
The passed props to the buttons need optimisation to eliminate redundancy, such as setting the size to 32 and passing the URL as url={data.link}.

idebenone · 2023-09-16T09:47:24Z

@medyo

new ui of the modal matching the settings modal.
removed the prop redundancy on share buttons
added parent class in css
added title prop for share buttons.

medyo · 2023-09-28T12:08:50Z

 }: CardItemWithActionsProps) => {
  const [showModal, setShowModal] = useState(false)
-  const [link, setLink] = useState({})
+  const [data, setData] = useState({})


data is too generic, please use a name like socialShareData or socialSharePayload

medyo · 2023-09-28T12:09:26Z

+const sharingButtons = [
+  { component: EmailShareButton, icon: FaMailBulk, title: 'Mail' },
+  { component: FacebookShareButton, icon: FaFacebookSquare, title: 'Facebook' },
+  { component: TwitterShareButton, icon: FaTwitterSquare, title: 'Twitter' },
+  { component: RedditShareButton, icon: FaRedditSquare, title: 'Reddit' },
+  { component: LinkedinShareButton, icon: FaLinkedinIn, title: 'Linkedin' },
+  { component: TelegramShareButton, icon: FaTelegram, title: 'Telegram' },
+  { component: WhatsappShareButton, icon: FaWhatsappSquare, title: 'Whatsapp' },
+]


Please you use colourful icons, they look much better

medyo · 2023-10-08T11:20:25Z

@idebenone the source text(LOBSTERS) looks brighter on light mode, please use the right css var, and keep it capitalised.

Thanks

idebenone · 2023-10-13T05:56:05Z

@medyo
Sorry for the delay and I have updated the css with right variable for source text and capitalized it

medyo · 2023-10-15T11:25:13Z

@idebenone I couldn't merge your branch, it has some conflicts with the develop branch, can you take a look?
thanks

idebenone · 2023-10-18T20:32:42Z

Hey @medyo
I just resolved the conflicts!
(I swear to god I almost had a heart attack going through it....my first time)

medyo · 2023-10-26T17:43:02Z

Hey @medyo I just resolved the conflicts! (I swear to god I almost had a heart attack going through it....my first time)

yeah, conflicts can be really draining, but you'll improve through practice.
I hope you're good.

I've given your code a test run, and it seems that the bookmark icon is no longer displaying.

medyo · 2023-10-26T17:55:32Z

I think it's a CSS issue, @victor-duarte can you take a look at this?

idebenone · 2023-10-26T18:28:15Z

@medyo
Yeah I just noticed....I'll try to see what's the problem

idebenone · 2023-10-26T19:00:12Z

.blockActionButton:nth-child(1) {
margin-right: 40px;
}

this will push the margin of bookmark icon from right making it visible on the screen.
Should I go with this or do I give new CSS class for both Block Action Buttons (bookmark & share)?

medyo · 2023-10-26T19:10:25Z

I think we need a scalable way to allow adding more than 2 buttons.
What about wrapping these buttons(share, bookmark...) with flexbox and then applying the slide/opacity animation to whole div instead of the button?

victor-duarte · 2023-10-27T01:12:47Z

👋🏻

Note: When enabling tab navigation an odd scroll was displaying when focusing on the CTA due to the container being outside the card and animating into the card. The animation was applied on the button instead and the container moved within the card preventing the layout shift.

My 2 cents would be to go with a simple solution that allows having the 2 buttons and if in the future a third button is needed maybe looking into alternative card designs. I am no designer but maybe something the line X or instagram.

e.g:

Suggestion

.blockActionButton {
  ...
  bottom: 40px;
  ...
}

.blockActionButton + .blockActionButton {
  bottom: 8px;
}
...
.blockRow:hover .blockActionButton,
.blockActionButton:focus-visible,
.blockActionButton:focus-visible ~ .blockActionButton, /* OPTIONAL: to improve tab navigation experience (subjective) */
.blockActionButton.active
.blockActionButton.active ~ .blockActionButton /* OPTIONAL: to display share icon when an item is book marked */ {
  ...
}

Hover will display the two buttons
Tab navigation will display the buttons independently (or partially together if optional line above is added in css). See attachments.

option-1.mov

option-2.mov

victor-duarte · 2023-10-27T01:05:49Z

 code {
  font-family: source-code-pro, Menlo, Monaco, Consolas, 'Courier New', monospace;
-}
+}


Revert and update touched files.

For consistency leave an empty line on the files (might be due to a local config in the IDE that the lines are being stripped out)

victor-duarte · 2023-10-27T01:06:54Z

+
      {cardItem}
      <div className={`blockActions ${isBookmarked ? 'active' : ''} `}>
        <button


Add aria-label="Bookmark item".

victor-duarte · 2023-10-27T01:07:37Z

          onClick={onBookmarkClick}>
          {!isBookmarked ? <BiBookmarkPlus /> : <BiBookmarkMinus />}
        </button>
+        <button className={`blockActionButton `} onClick={handleOpenModal}>


Add aria-label="Open share modal"

victor-duarte · 2023-10-27T01:19:23Z

-
+  const handleOpenModal = () => {
+    setShowModal(!showModal)
+    setShareData({ title: item.title, link: item.url, source: source })


Suggestion: avoid the. double state:

Use the shareData to handle the modal toggle

Reset the data when the modal closes

I don't know much about react best practices but it seems the double state is not needed since the modal depends on if data is selected or not.

victor-duarte · 2023-10-27T01:21:54Z

+
+.shareBody p {
+    font-size: 15px;
+    line-height: 1px;


Adjust element margins instead of setting line height to 1px

victor-duarte · 2023-10-27T01:25:50Z

+      overlayClassName="Overlay">
+      <div className="modalHeader">
+        <h1 className="modalTitle">Share</h1>
+        <button className="modalCloseBtn" onClick={handleCloseModal}>


Add aria-label="Close share modal"

victor-duarte · 2023-10-27T01:26:28Z

+        <h3>{shareData.title}</h3>
+        <div className="shareLink">
+          <input type="text" value={shareData.link} disabled />
+          <button onClick={copyLink}>


Add aria-label="Copy source url to clipboard"

victor-duarte · 2023-10-27T01:34:39Z

+      ariaHideApp={false}
+      shouldCloseOnEsc={true}
+      shouldCloseOnOverlayClick={true}
+      shouldFocusAfterRender={false}


Set this to true to improve a11y

Will allow the user to reach the modal if navigating with keyboard and close the modal when hitting ESC

Would suggest doing this on other modals also

victor-duarte · 2023-10-27T02:44:36Z

+    gap: 20px;
+}
+
+@media (max-width: 768px) {


Suggestion: if you follow a mobile first approach you could trim down a couple of lines:

.shareModal { background-color: var(--card-background-color); bottom: 0; box-shadow: 0 0 20px #00000052; box-sizing: border-box; left: 0; padding-bottom: 90px; padding: 12px 12px 90px; position: absolute; width: 100%; } ... @media (min-width: 768px) { .shareModal { border-radius: 10px; bottom: auto; left: 50%; padding-bottom: 12px; top: 50%; transform: translate(-50%, -50%); width: 450px; } .shareOptions { gap: 20px; } }

victor-duarte · 2023-10-27T02:54:12Z

+
+.shareLink {
+    display: flex;
+    flex-direction: row;


flex-row is the default direction, it can be removed.

victor-duarte · 2023-11-03T13:20:19Z

👋🏻 @medyo if permission are granted in this branch/PR I can apply the changes of the comments above.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

cc: @idebenone

medyo · 2023-11-03T13:54:47Z

@victor-duarte I don't see any setting to allow that permission on my side.
I think it's up to the PR owner to allow changes on his branch

victor-duarte · 2023-11-03T21:39:12Z

@idebenone, if a hand is needed please let me know if you are able to grant permissions to work on the PR.
Thanks!

idebenone · 2023-11-06T18:14:24Z

@idebenone, if a hand is needed please let me know if you are able to grant permissions to work on the PR. Thanks!

Hey @victor-duarte
I'm not able to find the option to allow collaborators on PR. I'll try to fix as many as possible(my hands are quiet tied rn).

idebenone · 2023-11-06T18:57:30Z

@victor-duarte

added all the aria-labels and made other css related changes that you had suggested related to shareModal
The modals doesnt close on ESC press even after setting ariaHideApp={true}. Need to look deeper into this.

idebenone · 2023-11-19T13:55:51Z

@medyo Could you please review my last commit?

medyo · 2023-11-19T18:24:58Z

@idebenone "To address issues with smaller post rows, let's switch the flex direction to 'row' with some gaps, The current implementation isn't fitting well.

Could you remove the position: absolute from the blockActionButton? We're currently managing the animation at the parent level

idebenone · 2023-11-20T05:09:54Z

@medyo

position: absolute removed. Pushed margin on left and bottom to give a nice lift to the buttons. Also changed the order of the elements. With bookmark button as the first element from left, if a post is saved it will look odd when the button is active.
If everything is cool, I'll commit the changes.

medyo · 2023-11-20T21:08:37Z

Yes, this looks much better. Feel free to commit the new changes

idebenone · 2023-11-21T14:57:54Z

@medyo Done 👍.

medyo · 2023-11-22T19:57:05Z

Thanks a lot @idebenone, You did great!

* Feature to share news/posts/articles (#162) * feature to share news/posts/articles * added title and source to share model * added title, removed redundancy of props * colorful icons & title prop added to button * css updated * css changes * fixed block-action-btns * Merge branch 'shareFeature' of github.com:idebenone/hackertab.dev * fix yarn.lock * persist fetched queries in localstorage * improve the share modal UI * track copy and share events * tag hackertabdev twitter account * add missing api url --------- Co-authored-by: Vineeth <[email protected]>

feature to share news/posts/articles

d081549

medyo linked an issue Sep 5, 2023 that may be closed by this pull request

Option to share articles/posts/news link on social media #161

Closed

medyo added the New feature Request a new language to add label Sep 5, 2023

added title and source to share model

6de9fb2

added title, removed redundancy of props

8ba721f

medyo reviewed Sep 28, 2023

View reviewed changes

colorful icons & title prop added to button

53bb46e

medyo changed the base branch from master to develop October 8, 2023 11:13

css updated

965d450

Merge branch 'develop' into shareFeature

a95ece8

victor-duarte reviewed Oct 27, 2023

View reviewed changes

css changes

2eea296

fixed block-action-btns

51ddb68

medyo merged commit 1ceae40 into medyo:develop Nov 22, 2023

Conversation

idebenone commented Sep 4, 2023

Uh oh!

medyo commented Sep 11, 2023

Uh oh!

idebenone commented Sep 11, 2023

Uh oh!

medyo commented Sep 14, 2023

Uh oh!

idebenone commented Sep 16, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

medyo commented Oct 8, 2023

Uh oh!

idebenone commented Oct 13, 2023

Uh oh!

medyo commented Oct 15, 2023

Uh oh!

idebenone commented Oct 18, 2023

Uh oh!

medyo commented Oct 26, 2023

Uh oh!

medyo commented Oct 26, 2023

Uh oh!

idebenone commented Oct 26, 2023

Uh oh!

idebenone commented Oct 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

medyo commented Oct 26, 2023

Uh oh!

victor-duarte commented Oct 27, 2023

Suggestion

Uh oh!

victor-duarte Oct 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

victor-duarte Oct 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

victor-duarte commented Nov 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

medyo commented Nov 3, 2023

Uh oh!

victor-duarte commented Nov 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

idebenone commented Nov 6, 2023

Uh oh!

idebenone commented Nov 6, 2023

Uh oh!

idebenone commented Nov 19, 2023

Uh oh!

medyo commented Nov 19, 2023

Uh oh!

idebenone commented Nov 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

idebenone commented Oct 26, 2023 •

edited

Loading

victor-duarte Oct 27, 2023 •

edited

Loading

victor-duarte Oct 27, 2023 •

edited

Loading

victor-duarte commented Nov 3, 2023 •

edited

Loading

victor-duarte commented Nov 3, 2023 •

edited

Loading

idebenone commented Nov 20, 2023 •

edited

Loading