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

MDC Migration: Merging feature branch with all new Angular Components Styled as desired. #6631

Merged
merged 19 commits into from
Oct 12, 2023

Conversation

JamesHollyer
Copy link
Contributor

Contributors: @rileyajones, @bmd3k, @JamesHollyer

Motivation for features / changes

Angular is deprecating the "Legacy" Material Components. We are migrating to the new MDC Components. This was done in a feature branch. This PR merges the feature branch into master with all components except for the mat-slider converted and styled properly.

Technical description of changes

This was all done in a feature branch for commit history please see that branch here: https://github.com/tensorflow/tensorboard/tree/MDCMigration

Screenshots of UI changes (or N/A)

Here is one basic screenshot of what a TensorBoard looks like now. For more detailed screenshots please see commit history with above link.
Screenshot 2023-10-10 at 1 50 06 PM

JamesHollyer and others added 17 commits September 28, 2023 00:24
## Motivation for features / changes
Background: We are trying to migrate to all the non legacy angular
components. To accomplish this we changed them all over at once and
began hunting for differences.

One of the biggest differences is that the header is totally foobar.
This seems to be primarily due to the following issues:
1) The header had a lot of custom hacky css in it
2) The `mat-tab` component had a significant number of structural
changes


## Screenshots of UI changes (or N/A)
### Before

![image](https://github.com/tensorflow/tensorboard/assets/78179109/2fcc3f96-bb14-4d41-a9b6-d225e81c0f28)

![image](https://github.com/tensorflow/tensorboard/assets/78179109/30e06383-a5dd-4b63-9f53-95fd378ee502)


### After

![image](https://github.com/tensorflow/tensorboard/assets/78179109/dc09e6fb-25e5-46ac-9a69-2b8b0ec41ad5)

![image](https://github.com/tensorflow/tensorboard/assets/78179109/faac633f-8072-4c2a-af8f-e77cf5afc77b)

### With Restyling

![image](https://github.com/tensorflow/tensorboard/assets/78179109/f248fb4d-3ae5-4e61-8e18-7f3deaf8fb46)


![image](https://github.com/tensorflow/tensorboard/assets/78179109/0f865db6-4977-4604-b8e4-4e341fbc33b9)

### Googlers
Light Mode https://screenshot.googleplex.com/AJdQE3T35aQSxpT
Dark Mode https://screenshot.googleplex.com/4Tn3XYqd6FSKCzS
## Motivation for features / changes
Updating the style of this menu after update to new MDC components. T
## Technical description of changes
The new MDC mat-menu no longer uses `<label>` and can also handle
mat-icons on its own.

## Screenshots of UI changes (or N/A)
<img width="173" alt="Screenshot 2023-09-28 at 12 02 41 PM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/57b3d6c7-4b43-484e-8668-03dc43081e27">

<img width="170" alt="Screenshot 2023-09-28 at 12 04 19 PM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/cd1ec903-9c05-463f-ac93-c97cdea8b0ee">
## Motivation for features / changes
After migrating to the new Material Components many of the styles were
broken. This cleans up the buttons in the runs table context menu.

## Screenshots of UI changes (or N/A)
<img width="196" alt="Screenshot 2023-09-28 at 2 59 22 PM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/01ef7e8e-7ea0-4de6-ae75-c9fc232e0549">
<img width="196" alt="Screenshot 2023-09-28 at 2 59 12 PM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/21842916-7805-4aa2-baec-8357ac5ec77f">
…ow#6614)

This restyles the right-hand pane dropdowns. The end result is:
1. selection trigger looks mostly like current.
2. selection panel is styled similar to current but selection rendering
is different and the panel is offset lower and slightly to the right.

Here are some sample screenshots but please patch it in locally and give
it a try:


![image](https://github.com/tensorflow/tensorboard/assets/17152369/9a03dbd2-36c2-4f82-aa0a-760dffdbb6f3)


![image](https://github.com/tensorflow/tensorboard/assets/17152369/8a7acd25-f099-4bd9-b6c2-180914d70262)
…6615)

Restyle the checkboxes in both the runs table and right pane.

For runs table this is mostly just about removing and adjusting padding.

For right pane this:
* adds negative margin to account for checkbox padding - aligning the
checkboxes with other elements in the pane.
* adjusting the text of checkbox labels to look more like the current
style.

Sample screenshots:

![image](https://github.com/tensorflow/tensorboard/assets/17152369/9f09d7b5-f708-469c-8eea-76b0ea1129a3)

![image](https://github.com/tensorflow/tensorboard/assets/17152369/fb35c457-e2c1-47e7-a03d-5d088a6cfbdb)
…tensorflow#6612)

This is another attempt at styling the plugins selector component for
MDC Migration.

We start by reverting the previous attempt so that we have a good base
for the conversion.
(See b846aae)

We then take the approach of attempting to adjust the current styling to
mdc components as much as possible, in order to preserve current styling
and current behavior.
(See 4012cae for the clean diff
compared to original)

The end result is:
* The header plugin tabs appear and behave almost identically to
current.
* The inactive plugin selector is styled slightly differently but still,
in spirit, is similar to current.

Sample screenshots:

![image](https://github.com/tensorflow/tensorboard/assets/17152369/e2a5c499-f17c-4539-92f9-c904cfcbb794)

![image](https://github.com/tensorflow/tensorboard/assets/17152369/740db4f3-ecf9-4f48-9f0a-4ade3db0cc3d)
## Motivation for features / changes
One of the biggest changes we have noticed during the mdc migration
appears in the `mat-icon-button`. I am opting to attempt to fix them all
at once by modifying the global files present in the theme file, then
tweaking specific buttons.

## Screenshots of UI changes (or N/A)
### Before
#### Light Mode

![image](https://github.com/tensorflow/tensorboard/assets/78179109/d1baa90d-ff69-441c-9239-9b91ba5a9d73)
#### Dark Mode

![image](https://github.com/tensorflow/tensorboard/assets/78179109/fb13873f-8462-45a2-bc51-b1eabc653da5)
##### Extent Button

![image](https://github.com/tensorflow/tensorboard/assets/78179109/fc30265c-bfde-4fdc-9dad-cb7ea2208072)
#### Table Header Buttons

![image](https://github.com/tensorflow/tensorboard/assets/78179109/4f5781de-d542-4f5e-8e25-37e8ed90746d)

### After
#### Light Mode

![image](https://github.com/tensorflow/tensorboard/assets/78179109/585da6c2-ff95-42c8-a0f0-a86f350edff3)
#### Dark Mode

![image](https://github.com/tensorflow/tensorboard/assets/78179109/6006cefe-05af-446d-b3ad-bdc69981fa87)
#### Extent Button

![image](https://github.com/tensorflow/tensorboard/assets/78179109/474a2e5a-c151-4aef-9743-a0010b712cc0)
#### Table Header Buttons

![image](https://github.com/tensorflow/tensorboard/assets/78179109/db336a4f-b0cd-4ae2-a421-4f11bb0b4533)

### After with Restyling
#### Light Mode

![image](https://github.com/tensorflow/tensorboard/assets/78179109/d6a23d2e-69ff-42e2-92b1-54de7f0af605)
#### Dark Mode

![image](https://github.com/tensorflow/tensorboard/assets/78179109/e36ba8ed-a7dd-4a42-a0d4-6bb6f6152983)
#### Extent Button

![image](https://github.com/tensorflow/tensorboard/assets/78179109/6a3b8a81-e432-4420-b8b5-3cb52827efd1)
#### Table Header Buttons

![image](https://github.com/tensorflow/tensorboard/assets/78179109/444ab6ec-c46f-4982-b892-19d137db5b70)

### Googlers see
[Light Mode](https://screenshot.googleplex.com/85bjNe5T2H6nBhy)
 [Dark Mode](https://screenshot.googleplex.com/5grTGURG8kddzfX)
tensorflow#6617)

Restyle the runs data table column selector.

The contents end up being larger than current master version.
The search icon, and search text input are bigger.
The height of each item in the list is taller to match the larger button
touch target.

Some sample screenshots:

![image](https://github.com/tensorflow/tensorboard/assets/17152369/fb8e2f5e-3170-4222-b356-58c333cdcb57)

![image](https://github.com/tensorflow/tensorboard/assets/17152369/47b8aecb-3c1f-4615-a968-7564b2f93bd7)
…ensorflow#6616)

Restyle the height of header and rows in the runs data table. 

The height of these rows does need to increase compared to current
master in order to account for the increase in height of the checkbox
touch target. But they don't need to be as tall as they are now compared
to current MDCMigration.

The end result is:
* Header and rows height are now 48px. (compared to 56px and 49px in
MDCMigration and compared to 48px and 44px in master). So the non-header
rows grow in height by 4px compared to master. Header rows remain the
same compared to master.

Implementation Detail:
* I move the common `padding: 4px` out of content_cell_component and
header_cell_component and now make this padding the responsibility of
runs_data_data and scalar_card_data_table to specify.
* scalar_card_data_table specifies, for the time being, `padding: 4px`
until we determine we need to make adjustments to it as well for MDC
migration.
* runs_data_table specifies different padding and adjust its height.

Some sample screenshots:

![image](https://github.com/tensorflow/tensorboard/assets/17152369/5b748e16-3c0d-40ad-a4df-8a400ec4e0e8)
…on. (tensorflow#6618)

Style the "settings" button and the "scalars | images | histograms"
plugin chooser button.

We can do this by adjusting the global styles we recently introduced,
meaning that other buttons we add in the future will also adhere to
these styles:
1. Set letter-spacing to normal for all button text.
2. Reuse the .mat-icon styling we introduced even for non icon-button
buttons.

Sample screenshot:

![image](https://github.com/tensorflow/tensorboard/assets/17152369/23c3ecda-93d3-4c73-8774-03379e2f802c)
## Motivation for features / changes
After an update to the new MDC Components the style of this feature was
messed up. This fixes that style.

## Screenshots of UI changes (or N/A)
<img width="458" alt="Screenshot 2023-10-05 at 3 03 57 PM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/6ddc2e53-f129-4fc2-b06e-7a7e1debdb59">
<img width="456" alt="Screenshot 2023-10-05 at 3 04 04 PM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/961bca54-d8b3-4f5c-961e-014ba0d1bdcd">
<img width="476" alt="Screenshot 2023-10-05 at 3 04 17 PM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/8c66d5ec-16aa-42fd-92b8-fdea3623c8bf">
## Motivation for features / changes
After migrating to the new MDC components many new components needed
style fixes. This is that fix for the SettingsDialogComponent.
## Screenshots of UI changes (or N/A)
before:
<img width="409" alt="Screenshot 2023-10-06 at 3 58 41 PM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/5f91c6b0-3941-4a9e-a4a1-48a779ead69a">

after:
<img width="400" alt="Screenshot 2023-10-06 at 3 20 11 PM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/4420a432-0092-428e-a5af-21e84ba4e8cb">
## Motivation for features / changes
Fixing regex since it looked pretty bad after MDC Migration. While doing
this I also standardized the padding across the entire page to 16px.

## Screenshots of UI changes (or N/A)
Before:
<img width="774" alt="Screenshot 2023-10-10 at 11 46 33 AM"
src="https://github.com/tensorflow/tensorboard/assets/8672809/dcb14b93-2afd-4f45-b269-a8b3d6c6d9b8">


After:
![Screenshot 2023-10-10 at 12 18 58
PM](https://github.com/tensorflow/tensorboard/assets/8672809/35cb2918-f5f7-4f97-8931-d4faf4b74798)
Copy link
Contributor

@rileyajones rileyajones left a comment

Choose a reason for hiding this comment

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

image

I believe you said that tap actually passed with cl/571955565

@JamesHollyer JamesHollyer merged commit 9c25f00 into tensorflow:master Oct 12, 2023
13 checks passed
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.

3 participants