Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

saved searches & prompt library fixes#63930

Merged
sqs merged 3 commits into
mainfrom
sqs/prompt-lib-more
Jul 19, 2024
Merged

saved searches & prompt library fixes#63930
sqs merged 3 commits into
mainfrom
sqs/prompt-lib-more

Conversation

@sqs

@sqs sqs commented Jul 19, 2024

Copy link
Copy Markdown
Member
  • Add spacing at bottom of page
  • Fix viewerCanAdminister check to return false not an error when the user is not an administrator of a saved search or prompt
  • Improve display of prompt description

Test plan

In dotcom mode, try loading a prompt or saved search in incognito mode.

@sqs sqs requested review from a team July 19, 2024 03:36
@cla-bot cla-bot Bot added the cla-signed label Jul 19, 2024
Comment thread client/web/src/prompts/Page.tsx Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

could the Page itself have a pb-4 so we don't have this div here just for styling? non-blocker.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

no, due to some weird margin collapse thing that involves a lot of css overflow stuff. I mean, yes, but I cheated here.

Comment thread client/web/src/prompts/PromptIcon.tsx Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

does that need more attribution than just this, i.e. by inlining the license?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

might be nice to change the return type since this function now never returns an error

Comment thread client/web/src/savedSearches/Page.tsx Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same question about adding pb-4 for the outer Page

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe change the return type to not include error since this function now never errors.

sqs added 3 commits July 19, 2024 01:23
- Add spacing at bottom of page
- Fix viewerCanAdminister check to return `false` not an error when the user is not an administrator of a saved search or prompt
- Improve display of prompt description
@sqs sqs force-pushed the sqs/prompt-lib-more branch from 30ae0bc to 7e2b3c0 Compare July 19, 2024 08:26
@sqs sqs enabled auto-merge (squash) July 19, 2024 08:26
@sqs sqs merged commit 2451aab into main Jul 19, 2024
@sqs sqs deleted the sqs/prompt-lib-more branch July 19, 2024 08:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants