-
Notifications
You must be signed in to change notification settings - Fork 462
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: pinned apps on top of all apps + rm list view switch #2699
Conversation
Branch preview✅ Deploy successful! |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
Coverage report
Show files with reduced coverage 🔻
Test suite run success1044 tests passing in 142 suites. Report generated by 🧪jest coverage report action from e15efc1 |
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 👍
src/pages/apps/bookmarked.tsx
Outdated
<SafeAppsSDKLink /> | ||
// Redirect to /apps | ||
useEffect(() => { | ||
router.push({ pathname: AppRoutes.apps.index, query: { safe: router.query.safe } }) |
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.
Should we use router.replace
instead so the user doesn't have to go back twice?
Looks good to me. The Bookmark Tab is gone Note: |
@francovenica they filter all apps together, pinned and unpinned mixed in one filtered list. |
What it solves
Removes the "Bookmarked apps" tab and instead shows pinned app on top of the main list.
Also we're removing the list/grid layout switch because of a very low usage. Last 60 days:
Design: https://www.figma.com/file/ptTs6lDBeUuLNySroJ5PiF/Web-Master-File?type=design&node-id=2102-55784&mode=design&t=9f1b7qvhnoHSaIFl-0