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

mgr/dashboard: Bootstrapify #35954

Merged
merged 3 commits into from Aug 10, 2020
Merged

mgr/dashboard: Bootstrapify #35954

merged 3 commits into from Aug 10, 2020

Conversation

SchoolGuy
Copy link
Contributor

This PR is directly based on #35501. Before that is merged, this one cannot be merged.

Checklist

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard backend
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@SchoolGuy SchoolGuy requested a review from a team as a code owner July 7, 2020 07:12
@SchoolGuy SchoolGuy marked this pull request as draft July 7, 2020 07:12
@SchoolGuy
Copy link
Contributor Author

The tests are failing because of unresolved paths for some pictures in the SCSS. Locally this just works fine. Anyone any ideas?

@SchoolGuy
Copy link
Contributor Author

jenkins test dashboard backend

@SchoolGuy SchoolGuy marked this pull request as ready for review July 24, 2020 08:32
@Exotelis Exotelis self-requested a review July 24, 2020 08:36
tspmelo
tspmelo suggested changes Jul 24, 2020
View reviewed changes
Copy link
Contributor

@tspmelo tspmelo left a comment

Choose a reason for hiding this comment

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

Didn't check every page, but cd-col-form-label is breaking the pages I tested.
Please check the other pages and maybe revert this.

I notice lots of colors have change throughout the dashboard.
It would be nice if you could show some screenshots of the bigger changes so everyone can comment on those.

for example the "No data to display" warning is now blue
image

And the selection row is now darker, and the warning text is brighter (contrast is not the best).
image

src/pybind/mgr/dashboard/frontend/src/app/ceph/dashboard/info-card/info-card.component.scss Outdated Show resolved Hide resolved
src/pybind/mgr/dashboard/frontend/src/app/shared/components/usage-bar/usage-bar.component.scss Outdated Show resolved Hide resolved
src/pybind/mgr/dashboard/frontend/src/styles/bootstrap-extends.scss Outdated Show resolved Hide resolved
src/pybind/mgr/dashboard/frontend/src/styles.scss Show resolved Hide resolved
src/pybind/mgr/dashboard/frontend/src/styles.scss Show resolved Hide resolved
src/pybind/mgr/dashboard/frontend/src/app/core/auth/login/login.component.scss Show resolved Hide resolved
src/pybind/mgr/dashboard/frontend/src/app/core/auth/login/login.component.scss Show resolved Hide resolved
src/pybind/mgr/dashboard/frontend/src/styles/defaults/_bootstrap-defaults.scss Outdated Show resolved Hide resolved
...nd/src/app/ceph/block/mirroring/bootstrap-create-modal/bootstrap-create-modal.component.html Outdated Show resolved Hide resolved
...nd/src/app/ceph/block/mirroring/bootstrap-import-modal/bootstrap-import-modal.component.html Outdated Show resolved Hide resolved
@SchoolGuy
Copy link
Contributor Author

jenkins test make check

@SchoolGuy
Copy link
Contributor Author

jenkins test dashboard

@SchoolGuy
Copy link
Contributor Author

jenkins test dashboard backend

@SchoolGuy
Copy link
Contributor Author

jenkins test make check

@SchoolGuy
Copy link
Contributor Author

jenkins test dashboard

@SchoolGuy
Copy link
Contributor Author

Here are the screenshots desired by @tspmelo

On the left side you see the new dashboard and on the right side the old dashboard. If you desire screenshots of other views please say which one you desire.

pr-35954-cluster-hosts-1
pr-35954-cluster-logs-1
pr-35954-cluster-osds-1
pr-35954-cluster-osds-edit-1
pr-35954-dashboard-1
pr-35954-pools-1
pr-35954-user-management-1

tspmelo
tspmelo suggested changes Aug 3, 2020
View reviewed changes
...ontend/src/app/ceph/block/mirroring/edit-site-name-modal/edit-site-name-modal.component.html Outdated Show resolved Hide resolved
...ontend/src/app/ceph/block/mirroring/pool-edit-mode-modal/pool-edit-mode-modal.component.html Outdated Show resolved Hide resolved
...ontend/src/app/ceph/block/mirroring/pool-edit-peer-modal/pool-edit-peer-modal.component.html Outdated Show resolved Hide resolved
...ontend/src/app/ceph/block/mirroring/pool-edit-peer-modal/pool-edit-peer-modal.component.html Outdated Show resolved Hide resolved
...ontend/src/app/ceph/block/mirroring/pool-edit-peer-modal/pool-edit-peer-modal.component.html Outdated Show resolved Hide resolved
...ard/frontend/src/app/ceph/cluster/mgr-modules/mgr-module-form/mgr-module-form.component.html Show resolved Hide resolved
...board/frontend/src/app/ceph/cluster/osd/osd-reweight-modal/osd-reweight-modal.component.html Outdated Show resolved Hide resolved
.../mgr/dashboard/frontend/src/app/shared/components/config-option/config-option.component.html Show resolved Hide resolved
src/pybind/mgr/dashboard/frontend/src/app/ceph/block/iscsi-setting/iscsi-setting.component.html Outdated Show resolved Hide resolved
...eph/block/iscsi-target-image-settings-modal/iscsi-target-image-settings-modal.component.html Outdated Show resolved Hide resolved
@SchoolGuy
Copy link
Contributor Author

@tspmelo Once again I squashed your recommendations in your review commit.

@votdev
Copy link
Member

votdev commented Aug 4, 2020

IMO the selected table rows color is too dark, the old one looks better, maybe something between those colors will be a good solution.

@alfonsomthd
Copy link
Contributor

@SchoolGuy Can you squash those commits that are not worth keeping apart?

@SchoolGuy
Copy link
Contributor Author

@alfonsomthd Sure thing. I will do it later in the day...

@alfonsomthd alfonsomthd added the skip-teuthology For PRs whose changes do not have an effect on QA runs/changes are not being tested in Teuthology label Aug 7, 2020
@SchoolGuy
Copy link
Contributor Author

SchoolGuy commented Aug 8, 2020 •
edited

Commits squashed, as per request of @alfonsomthd

tspmelo
tspmelo suggested changes Aug 10, 2020
View reviewed changes
Copy link
Contributor

@tspmelo tspmelo left a comment

Choose a reason for hiding this comment

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

commit mgr/dashboard: SCSS cleanup is missing the Fixes: https://tracker.ceph.com/issues/46382.

Please replace the message of commit mgr/dashboard: SCSS: Replaced usage of base class with specific with:

The SCSS class cd-col-form-label is the specific and correct class to use.
Previously sometimes the class col-form-label was used.

Fixes: https://tracker.ceph.com/issues/46382

Signed-off-by: Enno Gotthold <egotthold@suse.de>

@SchoolGuy
Copy link
Contributor Author

@tspmelo I force pushed and added the Fixes to my commit message.

@SchoolGuy
mgr/dashboard: SCSS cleanup
cdae818 
Removes unnecessary variables and uses already existing bootstrap variables instead.

Also we removed uneffective SCSS, as well as moved the Navtabs SCSS code into a module Bootstrap style

Last but not least we cleaned up toast & tooltip SCSS.

Fixes: https://tracker.ceph.com/issues/46382

Signed-off-by: Sebastian Krah <skrah@suse.com>
Signed-off-by: Enno Gotthold <egotthold@suse.de>
@SchoolGuy
mgr/dashboard: SCSS: Replaced usage of baseclass with specific
04d03c4 
The SCSS class cd-col-form-label is the specific and correct class to use. Previously sometimes the class col-form-label was used.

Fixes: https://tracker.ceph.com/issues/46382

Signed-off-by: Enno Gotthold <egotthold@suse.de>
@SchoolGuy
mgr/dashboard: Add a paragraph for the future Bootstrap 4
db9531f 
Fixes: https://tracker.ceph.com/issues/46382

Signed-off-by: Enno Gotthold <egotthold@suse.de>
@LenzGr LenzGr merged commit a77f46f into ceph:master Aug 10, 2020
votdev added a commit to votdev/ceph that referenced this pull request Aug 10, 2020
@votdev
Fix regression introduced by ceph#35954. Rename some color names that…
125ba82 
… have been changed by the mentioned PR.

Signed-off-by: Volker Theile <vtheile@suse.com>
votdev added a commit to votdev/ceph that referenced this pull request Aug 10, 2020
@votdev
Fix regression introduced by ceph#35954.
deca2cc 
Rename some color names that have been changed by the mentioned PR.

Signed-off-by: Volker Theile <vtheile@suse.com>
tchaikov pushed a commit to votdev/ceph that referenced this pull request Aug 10, 2020
@votdev @tchaikov
mgr/dashboard: correct color names which are not defined
828f2e9 
Fix regression introduced by ceph#35954.
Rename some color names that have been changed by the mentioned PR.

Signed-off-by: Volker Theile <vtheile@suse.com>
@bk201 bk201 mentioned this pull request Aug 17, 2020
Merged
3 tasks
@LenzGr LenzGr changed the title mgr/dashboard: Bootrapify mgr/dashboard: Bootstrapify Aug 26, 2020
ideepika pushed a commit to ceph/ceph-ci that referenced this pull request Sep 3, 2020
@votdev
mgr/dashboard: correct color names which are not defined
dcda061 
Fix regression introduced by ceph/ceph#35954.
Rename some color names that have been changed by the mentioned PR.

Signed-off-by: Volker Theile <vtheile@suse.com>
guits pushed a commit to guits/ceph that referenced this pull request Sep 16, 2020
@votdev @guits
mgr/dashboard: correct color names which are not defined
ef8f969 
Fix regression introduced by ceph#35954.
Rename some color names that have been changed by the mentioned PR.

Signed-off-by: Volker Theile <vtheile@suse.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LenzGr LenzGr LenzGr left review comments

@alfonsomthd alfonsomthd alfonsomthd approved these changes

@Exotelis Exotelis Exotelis approved these changes

@tspmelo tspmelo tspmelo approved these changes

@epuertat epuertat Awaiting requested review from epuertat

Assignees
No one assigned
Labels
dashboard needs-review skip-teuthology For PRs whose changes do not have an effect on QA runs/changes are not being tested in Teuthology
Projects
None yet
Milestone
No milestone
7 participants
@SchoolGuy @votdev @alfonsomthd @LenzGr @tspmelo @Exotelis @batrick