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.

Sign up for GitHub

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

Jump to bottom

Align Prefences #22940

Merged
merged 1 commit into from
Mar 23, 2020
Merged

Align Prefences #22940

merged 1 commit into from
Mar 23, 2020

Conversation

flopp999
Copy link
Contributor

@flopp999 flopp999 commented Mar 21, 2020 •
edited
Loading

What this PR does / why we need it:
Move Preferences little to the right/Preferences was more to the left then other menu items

Which issue(s) this PR fixes:
doesn't exist

Old view
image

new view
image

@flopp999
Align Prefences
e96389e 
Preferences was more to the left
@aknuds1
Copy link
Contributor

aknuds1 commented Mar 21, 2020 •
edited
Loading

Please write more explicitly what this PR does, and why it's needed, in the What this PR does / why we need it: section, so it's clear without reading the code.

@aknuds1 aknuds1 requested review from a team, mckn and hugohaggmark and removed request for a team March 21, 2020 10:56
@flopp999
Copy link
Contributor Author

Please write more explicitly what this PR does in the What this PR does / why we need it: section, so it's clear without reading the code.

I wrote that Preferences was more to the left and the pictures shows the difference.

@aknuds1 aknuds1 added pr/external This PR is from external contributor area/frontend labels Mar 21, 2020
@aknuds1
Copy link
Contributor

aknuds1 commented Mar 21, 2020

Please write more explicitly what this PR does in the What this PR does / why we need it: section, so it's clear without reading the code.

I wrote that Preferences was more to the left and the pictures shows the difference.

Yes, but that section should explain clearly what the PR does and why it's needed. "Preferences was more to the left" is very vague.

@flopp999
Copy link
Contributor Author

Please write more explicitly what this PR does in the What this PR does / why we need it: section, so it's clear without reading the code.

I wrote that Preferences was more to the left and the pictures shows the difference.

Yes, but that section should explain clearly what the PR does and why it's needed. "Preferences was more to the left" is very vague.

i updated the text now

mckn
mckn reviewed Mar 21, 2020
View reviewed changes
Copy link
Contributor

@mckn mckn left a comment

Choose a reason for hiding this comment

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

Looks good, I just need to do some more testing to see that it doesn't have a negative effect on other dropdowns before I can approve.

@torkelo
Copy link
Member

torkelo commented Mar 23, 2020

this will probably make something else miss-aligned, can you check all other menus? (main menu & panel menu),

we are moving to new icon library soon (instead of the mix we have now), should help alignment issues

@mckn
Copy link
Contributor

mckn commented Mar 23, 2020

this will probably make something else miss-aligned, can you check all other menus? (main menu & panel menu),

we are moving to new icon library soon (instead of the mix we have now), should help alignment issues

Have checked the following:

  • Dashboard search result ✅
  • Main menu ✅
  • Profile menu ✅
  • Help menu ✅
  • Graph panel tooltip menu ✅
  • Panel menu ✅

@mckn mckn self-requested a review March 23, 2020 10:25
mckn
mckn approved these changes Mar 23, 2020
View reviewed changes
Copy link
Contributor

@mckn mckn left a comment

Choose a reason for hiding this comment

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

Can't find any negative impact. Looks good!

@mckn mckn merged commit 8aca83a into grafana:master Mar 23, 2020
bergquist added a commit to AndrewBurian/grafana that referenced this pull request Mar 23, 2020
@bergquist
Merge branch 'master' into pd-severity-tag
0208865 
* master: (27 commits)
  Upgrade golangci-lint and fixes some linting errors. (grafana#22909)
  Storybook: Remove unneccesary stories and reorganize a bit (grafana#22959)
  Menu: aligning user menu options  (grafana#22940)
  Select: Fixed select text positition (grafana#22952)
  CircleCI: Pin grabpl to v0.2.0 (grafana#22906)
  Docs: fix broken link (grafana#22949)
  Variables: migrates ad hoc variable type to react/redux.  (grafana#22784)
  AuthProxy: Fixes bug where long username could not be cached (grafana#22926)
  Admin: Added Name to user list (grafana#22939)
  Docs: Update email notifier documentation (grafana#22919)
  update latest to 6.7.1 (grafana#22933)
  changelog 6.7.1 (grafana#22932)
  NewPanelEdit: Organise sidebar in tabs  (grafana#22870)
  Query components unsafe lifecycle methods (grafana#21163)
  Fix typo (grafana#22923)
  Panels: Fixed size issue with panels when existing panel edit mode (grafana#22912)
  Azure: Fixed dropdowns not showing current value (grafana#22914)
  Reporting: Update docs with new default value (grafana#22916)
  Fix: failing tests when running on none english os. (grafana#22913)
  BackendSrv: only add content-type on POST, PUT requests (grafana#22910)
  ...
bergquist added a commit that referenced this pull request Mar 23, 2020
@bergquist
Merge branch 'master' into document-jaeger-tracing-instrumentation
c8783bf 
* master: (733 commits)
  Alerting: Enable Alert rule tags to override PagerDuty Severity setting (#22736)
  Storybook: Fix broken stories (#22975)
  CircleCI: Reduce memory usage of golangci-lint (#22970)
  Fix mysterious Babel plugin errors (#22974)
  Variables: replaces homegrown variableAdapters with Registry (#22866)
  Fix typos in ha_setup.md (#22951)
  SAML: Document 2.0 as version (#22960)
  Variables: replaces UUID with name for a more performant lookup in TemplateSrv (#22858)
  Upgrade golangci-lint and fixes some linting errors. (#22909)
  Storybook: Remove unneccesary stories and reorganize a bit (#22959)
  Menu: aligning user menu options  (#22940)
  Select: Fixed select text positition (#22952)
  CircleCI: Pin grabpl to v0.2.0 (#22906)
  Docs: fix broken link (#22949)
  Variables: migrates ad hoc variable type to react/redux.  (#22784)
  AuthProxy: Fixes bug where long username could not be cached (#22926)
  Admin: Added Name to user list (#22939)
  Docs: Update email notifier documentation (#22919)
  update latest to 6.7.1 (#22933)
  changelog 6.7.1 (#22932)
  ...
@flopp999 flopp999 deleted the patch-1 branch March 30, 2020 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mckn mckn mckn approved these changes

@hugohaggmark hugohaggmark Awaiting requested review from hugohaggmark

Assignees
No one assigned
Labels
area/frontend pr/external This PR is from external contributor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@flopp999 @aknuds1 @torkelo @mckn