Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix field list with long terms #1137

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix field list with long terms #1137

wants to merge 2 commits into from

Conversation

Blendify
Copy link
Member

Currently, fieldlist extend the full content and may even overflow. Now with this patch, terms are limited to 20% of the field list.

This patch also refactors the margin to be handled by css grid gap property.

Possible future changes:

These are things that I have noticed that can be fixed in future patches.

  • Remove left padding -- seems unnecessary to me
  • Remove bottom margin on last child globally
  • Collapse grid on mobile

Currently, fieldlist extend the full content and may even overflow. Now with this patch, terms are limited to 20% of the field list.

This patch also refactors the margin to be handled by css grid gap property.

Possible future changes:

These are things that I have noticed that can be fixed in future patches.

- Remove left padding -- seems unnecessary to me
- Remove bottom margin on last child globally
- Collapse grid on mobile
@Blendify
Copy link
Member Author

Blendify commented Sep 1, 2022

Could this have a review, here is a bug report we got about this issue: https://developer.blender.org/T100721

Copy link
Contributor

@benjaoming benjaoming left a comment

Choose a reason for hiding this comment

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

This seems like the correct fix to me -- certainly, we could want it to wrap on mobile instead, but nonetheless this is an improvement of current behavior.

@benjaoming benjaoming added this to the 2.0 milestone Sep 1, 2022
@benjaoming
Copy link
Contributor

I think that @agjohnson is trying to wrap up 1.1 for an imminent release. So targeting this for 2.0. I know that it's just a small fix, but I think @agjohnson appreciates keeping the work load minimal for the release to come out.

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

Successfully merging this pull request may close these issues.

2 participants