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

Notes: load shared note based on the url parameter #343

Merged
merged 12 commits into from Sep 5, 2022

Conversation

OlegMoshkovich
Copy link
Member

@OlegMoshkovich OlegMoshkovich commented Aug 22, 2022

This PR is related to #314.
SideDrawer useEffect clears the issue number and moves the navpanel to the list view. .

@netlify
Copy link

netlify bot commented Aug 22, 2022

Deploy Preview for bldrs-share ready!

Name Link
🔨 Latest commit 8a512d3
🔍 Latest deploy log https://app.netlify.com/sites/bldrs-share/deploys/6315de9253895300095cb2d4
😎 Deploy Preview https://deploy-preview-343--bldrs-share.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@OlegMoshkovich OlegMoshkovich changed the title load shared not based on the url parameter load shared note based on the url parameter Aug 22, 2022
@OlegMoshkovich OlegMoshkovich self-assigned this Aug 22, 2022
@OlegMoshkovich OlegMoshkovich added the collab Collaboration Requirements label Aug 22, 2022
@pablo-mayrgundter
Copy link
Member

Is this description correct? Not sure what this is doing.. isn't it handling clearing behavior?

1 similar comment
@pablo-mayrgundter
Copy link
Member

Is this description correct? Not sure what this is doing.. isn't it handling clearing behavior?

@OlegMoshkovich OlegMoshkovich changed the title load shared note based on the url parameter Notes: load shared note based on the url parameter Aug 24, 2022
@OlegMoshkovich
Copy link
Member Author

@pablo-mayrgundter PTAL.

@pablo-mayrgundter
Copy link
Member

Hi, can you merge and now try a test for this?

@OlegMoshkovich OlegMoshkovich merged commit bd22bd2 into bldrs-ai:main Sep 5, 2022
OlegMoshkovich added a commit to OlegMoshkovich/Share that referenced this pull request Sep 5, 2022
* test issue number in the url

* add the logging for the extracted id

* debug the selection from the url parameter

* add logging to the side drawer

* move the logic for clearing the note to the side drawer

* subtract drawer open dependency from the drawer

* fix the logic in the useEffect

* clean up

* add side drawer issue body test

* fix the test

* clean up

* debug side drawer open comments use effect
@pablo-mayrgundter
Copy link
Member

Heya, let's try to avoid significant mods after Approval without another round of review. What ended up happening? Looks like the test for this got removed?

@OlegMoshkovich
Copy link
Member Author

I thought you approved it.

@pablo-mayrgundter
Copy link
Member

Per discussion, please restore test in a new PR. Thanks!

@OlegMoshkovich OlegMoshkovich deleted the noteLink branch September 27, 2022 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
collab Collaboration Requirements
Projects
No open projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

None yet

2 participants