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

[Assitants] Community | User tabs #773

Merged
merged 28 commits into from
Feb 8, 2024
Merged

[Assitants] Community | User tabs #773

merged 28 commits into from
Feb 8, 2024

Conversation

mishig25
Copy link
Collaborator

@mishig25 mishig25 commented Feb 4, 2024

Have a tab/picker between "community" or "my" assistants

Scenario 1: I'm not logged in

Screenshot 2024-02-04 at 6 37 45 PM

Scenario 2: I'm logged in as "mishig" & I go to "?assistants"
Screenshot 2024-02-04 at 6 37 56 PM

Scenario 3: I'm logged in as "mishig" & I go to "?assistants?user=victor"
Screenshot 2024-02-04 at 6 37 26 PM

Scenario 4: I'm logged in as "mishig" & I go to "?assistants?user=mishig"
Screenshot 2024-02-04 at 6 38 43 PM

@mishig25 mishig25 marked this pull request as draft February 4, 2024 18:26
@@ -34,5 +43,7 @@ export const load = async ({ url }) => {
assistants: JSON.parse(JSON.stringify(assistants)) as Array<Assistant>,
numTotalItems,
numItemsPerPage: NUM_PER_PAGE,
createdByUser: createdByName,
createdByMe: createdByName === (locals.user?.username ?? locals.user?.name),
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need for this I think, just compare the requested username with the current username in the page directly to see it's the same?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

handled in 6913adc

src/routes/assistants/+page.svelte Outdated Show resolved Hide resolved
@@ -13,6 +14,7 @@
import Pagination from "$lib/components/Pagination.svelte";

export let data: PageData;
export let user: LayoutData["user"];
Copy link
Collaborator

Choose a reason for hiding this comment

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

user is already in Pagedata (data.user)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

handled in f978c9d

@@ -90,6 +122,32 @@
<CarbonAdd />Create New assistant
</a>
</div>
{#if user?.username}
<div class="mt-10 flex gap-x-2">
{#if data.createdByUser && !data.createdByMe}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Something like this instead? createdByUser === data.user.username

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

handled in 6913adc

Base automatically changed from assistnats_pagination to main February 5, 2024 09:15
@mishig25
Copy link
Collaborator Author

mishig25 commented Feb 5, 2024

Updated behaviour.

for the info, I'm logged in as user "mishig"

Screen.Recording.2024-02-05.at.11.11.10.AM.mov

note: when I open "?user=victor". This is what I see. Once I click to "My assistants", "Victor's assissntas" are gone & turns into "Community"

Screen.Recording.2024-02-05.at.11.11.44.AM.mov

@mishig25 mishig25 marked this pull request as ready for review February 5, 2024 10:33
@mishig25 mishig25 requested review from gary149 and nsarrazin February 5, 2024 10:35

// fetch the top assistants sorted by user count from biggest to smallest, filter out all assistants with only 1 users. filter by model too if modelId is provided
const filter: Filter<Assistant> = {
userCount: { $gt: 1 },
modelId: modelId ?? { $exists: true },
featured: true,
createdByName: createdByName ?? undefined,
Copy link
Collaborator

Choose a reason for hiding this comment

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

it's doesn't seems to work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this specific filter ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mishig25 mishig25 requested a review from gary149 February 5, 2024 12:51
@@ -25,6 +27,19 @@
}
goto(newUrl);
};

function getHref(keysAdd: { key: string; val: string }[], keysDelete?: string[]) {
Copy link
Collaborator Author

@mishig25 mishig25 Feb 5, 2024

Choose a reason for hiding this comment

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

todo: should take the url as argument like in 8cd7b98

(not for myself. in subseq PR, I should refactor getHref as a utility function that can be used in different components)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

handled in 291a9be & 941380b

@mishig25
Copy link
Collaborator Author

mishig25 commented Feb 7, 2024

if this pr is taking long time, maybe we can just cherry-pick & merge 9a21527 to main ?

@nsarrazin
Copy link
Collaborator

What's the blocker for the PR? 👀

@gary149
Copy link
Collaborator

gary149 commented Feb 7, 2024

What's the blocker for the PR? 👀

I've updated it a bit, it should be ready now.

@mishig25
Copy link
Collaborator Author

mishig25 commented Feb 7, 2024

Right now when you are on some user's or your assistant (i.e. the url has ?user=xyz) & choose a model from dropdown, it removes user from searchQuery. Should the user persist in searchQuery in this case or not?

@nsarrazin nsarrazin added the assistants Related to the assistants feature label Feb 7, 2024
@mishig25
Copy link
Collaborator Author

mishig25 commented Feb 7, 2024

Updates:

  1. Refactored getHref utility func [Assitants] Community | User tabs #773 (comment)
  2. Regarding this comment, made this commit b7ede7f which preserves the user as model id changes

@mishig25
Copy link
Collaborator Author

mishig25 commented Feb 7, 2024

Please let me know if there's anything else

@mishig25
Copy link
Collaborator Author

mishig25 commented Feb 8, 2024

Commit c41f235

Before; it was showing single "Community" btn for non-logged in users

Screenshot 2024-02-08 at 10 58 43 AM

After c41f235; non-logged in users won't see "Community" btn. Only logged-in users would see "Community" btn AND "their name" btn

Screenshot 2024-02-08 at 10 59 48 AM

@mishig25 mishig25 merged commit 08e8583 into main Feb 8, 2024
3 checks passed
@mishig25 mishig25 deleted the assistants_by_me branch February 8, 2024 10:13
@@ -0,0 +1,41 @@
export function getHref(
Copy link
Member

@coyotte508 coyotte508 Feb 20, 2024

Choose a reason for hiding this comment

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

IMO easier to understand like this:

/**
 * Util to edit a URL's search params
 *
 * Returns the new URL
 */
export function editSearchParams(
	url: URL | string,
	modifications: {
		/** New search params added */
		add?: Record<string, string | undefined | null>;
		/** Remove search params */
		remove?: string[];
		/** Remove all existing search params except those */
		keepOnly?: string[];
	}
): string {

ice91 pushed a commit to ice91/chat-ui that referenced this pull request Oct 30, 2024
* [Assistants] Add pagination

* [Assitants] Community | User tabs

* format

* minimize diff

* Update src/routes/assistants/+page.svelte

Co-authored-by: Victor Muštar <victor.mustar@gmail.com>

* use data.user

* rm unused import

* Use a href & simplify overall

* lint

* it will never be a button

* statements need to be reactive

* format

* set filter differntly

* Revert "set filter differntly"

This reverts commit fc8a2fc.

* use inline

* avoid ugly image change

* fix query

* ui update

* update link to profiles

* link to HF profile

* dark mode

* Update +page.svelte

* refactor getHref to be more descriptive

* Preserve `user` searchQuery when chaning model ids

* `getHref` utility

* dont show single "Community" btn for non-logged in users

* fix iterator

---------

Co-authored-by: Victor Muštar <victor.mustar@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assistants Related to the assistants feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants