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

Conditionally Show Attachment Button in Main Column #320

Closed

Conversation

jasonpaulso
Copy link
Contributor

Showing Attachment button only when assistant allows for image upload.

Jason Schulz added 2 commits May 3, 2024 10:58
…t name fixes: Conditionalizing attachment icon in composer is broken AllYourBot#319
<% if @assistant.images%>
<%= button_tag type: "button",
id: "attach-button",
class: "absolute left-11 bottom-[26px] w-[23px] h-[23px] relationship:hidden",
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't have to conditionalize this in ruby. I think it would normally be totally fine to solve it this way, but now that I'm reviewing this code I realize that I shied away from it only because the presence of assistant.images leads to two changes: (1) hiding the paperclip and (2) changing the padding on text input.

If it was only hiding the paperclip, I also would have just added an if since that's a lot more visible. But because it was two things I opted for a CSS approach rather than a ruby approach.

If you look at <div id="composer" ... a few lines above you can see relationship class being applied here. I made this up, but relationship is a word I invented which completely copies the group pattern in Tailwind. Meaning, when the relationship class is applied to a parent element, then any child elements with relationship:something get that style applied.

Right here where this comment is is the relationship:hidden so that should be causing this to disappear without any ruby. And if you look further down around line 246 is relationship:pl-4 on the text input which is changing the indentation. That one is definitely being applied, but I think it's this relationship:hidden that isn't working any longer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see what you are saying. I'll look into the issue with the hidden class.

@jasonpaulso jasonpaulso closed this May 6, 2024
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