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.

Sign up for GitHub

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

Jump to bottom

Fix: Search results all show as files #914

Merged
merged 6 commits into from
Apr 12, 2021
Merged

Fix: Search results all show as files #914

merged 6 commits into from
Apr 12, 2021

Conversation

FSM1
Copy link
Contributor

@FSM1 FSM1 commented Apr 11, 2021

Closes #883

@render
Copy link

render bot commented Apr 11, 2021

Your Render PR Server URL is https://chainsafex-dev-pr-914.onrender.com.

Follow its progress at https://dashboard.render.com/static/srv-c1pnmtbs1gh5cdub2okg.

@render
Copy link

render bot commented Apr 11, 2021

Your Render PR Server URL is https://files-ui-dev-pr-914.onrender.com.

Follow its progress at https://dashboard.render.com/static/srv-c1pnmu3s1gh5cdub2oq0.

@render
Copy link

render bot commented Apr 11, 2021

Your Render PR Server URL is https://files-landing-dev-pr-914.onrender.com.

Follow its progress at https://dashboard.render.com/static/srv-c1pnmujs1gh5cdub2pe0.

@FSM1 FSM1 added the Status: Review Needed 👀 Added to PRs when they need more review label Apr 11, 2021
@RyRy79261
Copy link
Contributor

https://github.com/ChainSafe/files-api/issues/924

This patch might not be required

Tbaut
Tbaut reviewed Apr 12, 2021
View reviewed changes
Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

Thanks a lot for digging into this. This is an api error from what I can tell from the conversation and this content type should not have landed in the UI originally. I am for getting the api fix in, and not introduce (and then have to delete) this directory2. Unless it takes forever for the api to be fixed, but it sounds like they have a clue where it comes from.

FSM1 and others added 2 commits April 12, 2021 14:00
@FSM1 @RyRy79261
Update packages/files-ui/src/Utils/Constants.ts
fa090bb 
Co-authored-by: Ryan Noble <ryanjnoble@gmail.com>
@FSM1 @RyRy79261
Apply suggestions from code review
98083ba 
Co-authored-by: Ryan Noble <ryanjnoble@gmail.com>
tanmoyAtb
tanmoyAtb approved these changes Apr 12, 2021
View reviewed changes
Copy link
Contributor

@tanmoyAtb tanmoyAtb left a comment

Choose a reason for hiding this comment

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

Just the two suggestions!
Looks good to go!

@tanmoyAtb
Copy link
Contributor

I'm for keeping the Directory2! and have an issue to remove it once the API gets fixed!

FSM1 and others added 2 commits April 12, 2021 17:35
@FSM1 @tanmoyAtb
Update packages/files-ui/src/Components/Modules/SearchModule.tsx
4f7d60b 
Co-authored-by: Tanmoy Basak Anjan <tanmoy3399@gmail.com>
@FSM1 @tanmoyAtb
Update packages/files-ui/src/Components/Modules/SearchModule.tsx
74b16b2 
Co-authored-by: Tanmoy Basak Anjan <tanmoy3399@gmail.com>
@FSM1 FSM1 mentioned this pull request Apr 12, 2021
Closed
@FSM1 FSM1 requested review from RyRy79261 and Tbaut April 12, 2021 15:41
@FSM1
Copy link
Contributor Author

FSM1 commented Apr 12, 2021

@Tbaut @RyRy79261 I've created an issue to remove the Directory2 ref when the API fix is released.

Tbaut
Tbaut approved these changes Apr 12, 2021
View reviewed changes
Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

With an issue, I'm good :)

@FSM1 FSM1 dismissed RyRy79261’s stale review April 12, 2021 16:35

will remove later

@FSM1 FSM1 merged commit 78c7241 into dev Apr 12, 2021
@FSM1 FSM1 deleted the fix/search-results-all-show-as-files-883 branch April 12, 2021 16:36
@FSM1 FSM1 mentioned this pull request Apr 12, 2021
Merged
FSM1 added a commit that referenced this pull request Apr 12, 2021
@FSM1
Merge pull request #908 from ChainSafe/dev
0392f31 
Crash when going back after having selected Google auth #860 (PR #871)
Login optimizations (PR #879)
Minor fixes + Updates to color manipulator (#874)
Merge pull request #879 from ChainSafe/fix/login-optimizations
Merge pull request #887 from ChainSafe/fix/tbaut-hover-color
Merge pull request #886 from ChainSafe/feat/tkey-darkmode
Remove code duplication (#889)
Merge pull request #895 from ChainSafe/fix/select-all-892
Merge pull request #896 from ChainSafe/fix/mouse-highlight-issue-890
Merge pull request #894 from ChainSafe/fix/mobile-spinner-893
Merge pull request #877 from ChainSafe/feat/tbaut-settings-security-846
folder validation (#899)
saved browsers settings (#865)
Prevent the whole app from rerendering every 5s (#909)
Merge pull request #911 from ChainSafe/fix/share-transfer-modal-on-ta… …
Merge pull request #905 from ChainSafe/fix/bin-file-view-873 …
Merge pull request #906 from ChainSafe/feat/home-folder-nav-885 …
Merge pull request #907 from ChainSafe/fix/search-bucket-to-reset-on-…
Fix: Search results all show as files (#914) …
Fix double click opening the next file preview (#915) …
Rename tooltip and fixes (#912) …
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tbaut Tbaut Tbaut approved these changes

@tanmoyAtb tanmoyAtb tanmoyAtb approved these changes

@RyRy79261 RyRy79261 Awaiting requested review from RyRy79261

Assignees
No one assigned
Labels
Status: Review Needed 👀 Added to PRs when they need more review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Search results all show as files
4 participants
@FSM1 @RyRy79261 @tanmoyAtb @Tbaut