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

Use boxes for system tags, shorten permission text #21958

Merged
merged 1 commit into from
Jan 28, 2016

Conversation

PVince81
Copy link
Contributor

Permission text now doesn't appear when all permissions are there, or
shows as "invisible" or "not assignable", which should better cover all
use cases.

Changed select2 style to use boxes in the input field.

As admin:
systemtags-boxes-admin

As user:
systemtags-boxes-user

Please review @nickvergessen @jancborchardt

@jancborchardt feel free to use your styling hammer brush and push on this PR (separate PR is fine too)
Note: we'll also have to talk about how to deal with wrapping.

@PVince81 PVince81 added this to the 9.0-current milestone Jan 27, 2016
@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @nickvergessen to be a potential reviewer

@karlitschek
Copy link
Contributor

nice 👍

@MorrisJobke
Copy link
Contributor

Tested and works, only one minor thing:

  • the cursor is a bit to narrow and it would be nice to have the cursor vertically centered.

bildschirmfoto 2016-01-27 um 16 50 42

scope = t('core', 'not assignable');
}
if (!tag.userVisible) {
// invisible also implicitly means not assignable
Copy link
Contributor

Choose a reason for hiding this comment

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

right, forgot to check that in the tag manager

@PVince81
Copy link
Contributor Author

@MorrisJobke yeah I probably removed one margin too much. Was expecting @jancborchardt to do his styling magic anyway.

@@ -62,16 +62,12 @@
}

.systemtags-select2-container .select2-choices .select2-search-choice {
Copy link
Contributor

Choose a reason for hiding this comment

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

just remove this completly? Or whats the problem with it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need the padding-left because normally select2 has little "close" buttons, but we removed them.
So the padding is needed.

Permission text now doesn't appear when all permissions are there, or
shows as "invisible" or "not assignable", which should better cover all
use cases.

Changed select2 style to use boxes in the input field.
@PVince81
Copy link
Contributor Author

I fixed the input field, it looks better now.

I also did some experiments with collapsing the field locally, but there's trouble with the dropdown position and it doesn't feel right. Will continue the collapsing this separately.

Please review.

@nickvergessen
Copy link
Contributor

👍 looks fancy and nice

@MorrisJobke
Copy link
Contributor

Yep 👍

DeepDiver1975 added a commit that referenced this pull request Jan 28, 2016
Use boxes for system tags, shorten permission text
@DeepDiver1975 DeepDiver1975 merged commit de8852a into master Jan 28, 2016
@DeepDiver1975 DeepDiver1975 deleted the systemtags-style branch January 28, 2016 11:54
@jancborchardt
Copy link
Member

Very cool, good fixes @PVince81! :)

@lock lock bot locked as resolved and limited conversation to collaborators Aug 8, 2019
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.

7 participants