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

Remove right margin from custom scrollbar #1498

Merged

Conversation

rotemdan
Copy link
Contributor

Here's a basic pull request for an attempt at fixing #1492

You can use it as a starting point. It's not tested (tried to run npm run preview locally but it failed due to lack of MongoDB, seems like getting this to work isn't trivial for me and it'd take some more time and effort than I have right now).

I simply removed the tailwind class mr-1, which means:

.mr-1 {
  margin-right: .25rem;
}

From instances of custom-scrollbar in these source files:

src/lib/components/chat/ChatWindow.svelte
src/routes/assistants/+page.svelte
src/routes/models/+page.svelte
src/routes/tools/+page.svelte

You'll need to test it yourself to ensure it actually works.

@rotemdan
Copy link
Contributor Author

rotemdan commented Sep 28, 2024

I've managed to start it locally using a docker container for the MongoDB instance.

The local instance doesn't fully work (getting errors like Authorization header is correct, but the token seems invalid) but I could paste a large amount of text to the chat input to see how the scrollbars behave.

Based on what I've seen, it seems the change does apply correctly.

Just note that Chromium has an unrelated issue with scrollbars: on my PC, for a zoom level of 125%, the scrollbar isn't pressable at the right edge, but with other zoom levels (100%, 150%, etc.) it does work. I've seen this happen on many other websites. It could be some internal issue with rounding of fractional pixels, or something of that sort. I'm not sure if it's actually reported on the Chromium tracker.

Copy link
Collaborator

@nsarrazin nsarrazin left a comment

Choose a reason for hiding this comment

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

Works great! Thanks for taking the time to create a PR 🤗

@nsarrazin nsarrazin merged commit 5e766fb into huggingface:main Oct 1, 2024
3 checks passed
ice91 pushed a commit to ice91/chat-ui that referenced this pull request Oct 30, 2024
Co-authored-by: Nathan Sarrazin <sarrazin.nathan@gmail.com>
@rotemdan
Copy link
Contributor Author

Issue has now returned. I'm not sure exactly when, but I think even yesterday it still worked correctly.

Looking at the updated source code, the files, where I applied changes to remove the mr-1 classes from tags, have changed significantly.

I see the mr-1 and mr-1.5 classes added in many different places. The code looks very different from when I made the changes previously.

Figuring out what changes to make to fix the issue was hard enough the last time. I don't know if I could go through it again. As a maintainer of many different open-source repositories myself, I'd never ask a user for a second pull request on an issue they correctly fixed earlier, if the reason it broke again is not related to them.

I reopened #1492.

As you can see, I personally really care about this particular behavior. I noticed it very quickly, and it's frustrating to have it back again.

@rotemdan
Copy link
Contributor Author

Turns out the return of the issue was actually a Chromium bug I activated by accidentally changing to a 125% zoom level. It wasn't coming from the website. I closed the issue again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants