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

Admin link #7851

Merged
merged 14 commits into from
Dec 14, 2020
Merged

Admin link #7851

merged 14 commits into from
Dec 14, 2020

Conversation

agriffard
Copy link
Member

Fixes #7806

Separate the upper left infos of the admin in two links:

  • The logo is a link to the admin.
  • The title is a link to the site.

@hishamco
Copy link
Member

hishamco commented Dec 6, 2020

I like the changes based on the screenshot, hope @Skrypt like it too

@agriffard agriffard requested review from Skrypt and removed request for Skrypt December 9, 2020 10:42
Copy link
Contributor

@Skrypt Skrypt left a comment

Choose a reason for hiding this comment

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

image

There is an extra margin there that looks odd between the user button and the 2 others. One of them is missing cursor:hand which can be fixed by adding role="button" on the anchor. The tenant name now has a blue color which I think shouldn't. User button should be the last one.

See Github header :

image

@agriffard
Copy link
Member Author

ok, UserMenu moved to navbar-collapse => margin removed.

agriffard and others added 2 commits December 13, 2020 22:01
Co-authored-by: Jasmin Savard <jasmin.savard@gmail.com>
Co-authored-by: Jasmin Savard <jasmin.savard@gmail.com>
@agriffard
Copy link
Member Author

image

Co-authored-by: Jasmin Savard <jasmin.savard@gmail.com>
@agriffard
Copy link
Member Author

With Signed in as:

image

@agriffard agriffard requested a review from Skrypt December 14, 2020 13:53
@Skrypt
Copy link
Contributor

Skrypt commented Dec 14, 2020

The only thing "buggy" is the tooltip on the dark/light mode button. We would need to be able to create translations for javascript.

@agriffard
Copy link
Member Author

The only thing "buggy" is the tooltip on the dark/light mode button. We would need to be able to create translations for javascript.

Why does it seems buggy according to you?

@Skrypt
Copy link
Contributor

Skrypt commented Dec 14, 2020

When I click on it ; the Tooltip title doesn't toggle from dark to light, vice versa.
That's a small issue that could be fixed later on. You can still merge this PR.

Now you assign the tooltip from server side depending on the actual mode it is but it can be changed on client side ...
We need to be able to pass translations to javascript vars for this to work because else these tooltip titles would be hardcoded in javascript which is not what we want. So "Light Mode" and "Dark Mode" should be translatable.

And the tooltip text should be changed in darkmode.js file of TheAdmin theme by changing the anchor title attribute. I think you changed this because previously it was using data-title-something.

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.

Admin Title
3 participants