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

Correctly align the settings label #2210

Closed
wants to merge 1 commit into from

Conversation

raimund-schluessler
Copy link
Contributor

After:
Screenshot 2021-09-06 at 21-23-41 Tasks - Nextcloud

Before:
Screenshot 2021-09-06 at 21-24-43 Tasks - Nextcloud

Closes #2138

Signed-off-by: Raimund Schlüßler <[email protected]>
@raimund-schluessler raimund-schluessler added bug Something isn't working 3. to review Waiting for reviews design Design, UX, interface and interaction design labels Sep 6, 2021
Comment on lines +83 to +86
#app-settings-header .settings-button {
padding-left: 50px;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I would argue the opposite, should be fixed in files, no?
What is the AppNavigationItem using?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not Files, this is Tasks and Calendar and literally every other app which uses AppNavigationItem. These 6 px here:
https://github.com/nextcloud/nextcloud-vue/blob/master/src/components/AppNavigationItem/AppNavigationItem.vue#L520
add to the 44 px icon width, which make it 50 px in total.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was added in this commit 5e7e395

Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

Maybe add a opacity: 1 !important; so that the opacity is the same like for the trashbin?

Also, it currently looks like this on c.nc.c
image
so not sure if this really needs fixing (or did you overwrite the stile already in the tasks app?

@raimund-schluessler
Copy link
Contributor Author

raimund-schluessler commented Sep 6, 2021

Maybe add a opacity: 1 !important; so that the opacity is the same like for the trashbin?

This style is coming from server, I didn't want to mess with it, especially not in this tiny PR.

Also, it currently looks like this on c.nc.c
image
so not sure if this really needs fixing (or did you overwrite the stile already in the tasks app?

I overwrote this style in the Tasks app (see here https://github.com/nextcloud/tasks/blob/005cd27fda7ce5f53d65c469bb254458fe3abe0d/src/views/AppNavigation.vue#L456-L458) because this misalignment annoyed me 🙈 This PR here upstreams this fix. Checkout the Calendar or Contacts app, there it is off.

@raimund-schluessler
Copy link
Contributor Author

Superseded by #2211.

@raimund-schluessler raimund-schluessler deleted the fix/settings-label branch September 6, 2021 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working design Design, UX, interface and interaction design
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Settings label not aligned to app-navigation-entry
3 participants