-
Notifications
You must be signed in to change notification settings - Fork 146
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
ui:(platform): fix spacing issue in the collections panel #101
base: main
Are you sure you want to change the base?
ui:(platform): fix spacing issue in the collections panel #101
Conversation
|
Name | Link |
---|---|
🔨 Latest commit | f666e06 |
It looks great, Congrats on your first PR @0xatulpatil. I have one thought about the collection UI, How about making it more configurable like a right/left aligned view? like this image, the methods are right aligned. if it's easy then we can implement it. and add a hardcode comment on how to change the alignment. later we'll create an app configuration UI where the user will be able to select the alignment by themself. what are your thoughts? @0xatulpatil |
afa7557
to
ab18ad3
Compare
That would be great!
did you mean left-aligned? Right now, I have attached a Also as mentioned in the issue, I'll make changes and make the prefix length to the |
my bad, yes the image is left-aligned. it's fine to include it within the same PR. also instead of custom class, i prefer to achieve it with tailwind classes first. |
"text-right": !leftAligned, | ||
}); | ||
const requestTokenClass = cx({ | ||
"pr-5": leftAligned, |
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.
these classes not begin generated by tailwind.
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.
try pnpm build:tailwind
at the platform/firecamp-ui
directory
"pr-5": leftAligned, | ||
"pl-5": !leftAligned, | ||
"w-11": true, | ||
"mr-2":true, |
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.
mr-2
class is generated tho
Hey @Nishchit14, I made the required changes, but I can figure out why some of my tailwind classes are not rendering. In the latest commit, I have added some comments, please check. I don't know if this has something to do with the tailwind config or the way cx handles the classes that are not indexed by tailwind. |
@0xatulpatil I am preparing the new version currently, and I am thinking of including this PR in the next release, thus I am putting this PR on hold as of now. also, you can run |
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!!
once you confirm that the tailwind classes are being generated and ui is fine with that then I'll approve and merge it.
closes #93
Changes
Fixed the margin-spacing issue with the collections tab.
The icons have the widht of the largest request-token which happens to be "OPTIONS" token.
Before:
#After