-
-
Notifications
You must be signed in to change notification settings - Fork 35
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
Add speech bubbles to citations in the user story page #84
Add speech bubbles to citations in the user story page #84
Conversation
Hi Sir @krisstern , There are 7 CI checks including deployment , but why are there been only 4 CI checks in this PR ; and also no deployment has been shown . |
Hi, you do not have the permissions to deploy your PR to the preview, not unless there is some input on the maintainers' (such as mine) side. |
Hi Sir, Thank you for clarifying! :) |
I have resolved conflicts here too ! |
Hi, before I can review this one final time, could you please resolve the conflicts in this PR? |
Sure Sir, doing it now and will update you soon ! |
Hi Sir , I want to point you out an important update, the PR #85 has only prettier changes and the PR #80 has the technical changes which excluded the prettier inclusion . Now since #85 got merged before #80 , the prettier inclusion got away from our main repo. So I wanted to ask you should I again have to raise the prettier inclusion PR ? Please let me know how I can help it out . Thanks ! |
Hello Sir , I have resolved the conflicts . |
Hi, |
Sure Sir , will do it now . I tried to resolve in vs code "merge conflicts" feature which might be not as good as needed. |
Hello Sir, I have made it through removing first and then installing |
Is there any more changes needed Sir for the PR ,please let me know about it too . |
Hi @biru-codeastromer your npm i Then commit and push your changes again. Thanks! |
Sure Sir ! Will be doing it now and confirm you here about update . |
Hello Sir, I have reinstalled the dependencies with Node.js version 20 with your steps as requested. Please let me know if further changes are needed and also if the |
package.json
Outdated
@@ -16,6 +16,7 @@ | |||
"lint": "eslint ." | |||
}, | |||
"dependencies": { | |||
"@parcel/watcher": "^2.5.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"@parcel/watcher": "^2.5.0", |
I am wondering why we need this new "@parcel/watcher"
dependency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please update the package-lock.json
file after the removal of this dependency locally as well before pushing to GitHub?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering why we need this new
"@parcel/watcher"
dependency?
Actually I didnt mean to include it but in my gatsby build
i got error due to absence of this for which I included and got my error removed from build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please update the
package-lock.json
file after the removal of this dependency locally as well before pushing to GitHub?
Okay Sir, doing it and inform you about the update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello Sir, I investigated the presence of @parcel/watcher
in package-lock.json. It appears as a transitive dependency required by gatsby via @parcel/cache
and @parcel/fs
. Since it is not explicitly listed in package.json
and is required for gatsby to function, it cannot be removed directly. Please let me know if any further action is needed. Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello Sir, However, since Please let me know if any further adjustments are needed! Thank you. |
Yes, I have checked we do not need this package in the "package.json" file for the build to compile. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks @biru-codeastromer for the PR!
Welcome Sir ! 😄 |
Fixes #8
Summary:
::after
pseudo-element.bottom
andborder-width
properties to achieve the desired effect.Changes Made:
.speechBubble
class**: Addedposition: relative;
for proper placement of the caret..speechBubble::after
pseudo-element**: Added a caret effect using borders to simulate the tail of the speech bubble.bottom
to position the caret slightly below the speech bubble.border-color
to match the speech bubble’s background.Issue Resolved:
Result:
Before :
After :
Please review the changes and let me know if there are any questions or further adjustments needed.