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

Feature/#340 UI scroll refactor #342

Merged
merged 31 commits into from
Sep 7, 2021
Merged

Conversation

iguannalin
Copy link
Contributor

No description provided.

@iguannalin iguannalin self-assigned this Aug 24, 2021
@iguannalin iguannalin linked an issue Aug 24, 2021 that may be closed by this pull request
11 tasks
@iguannalin iguannalin marked this pull request as ready for review August 31, 2021 18:23
@iguannalin iguannalin requested a review from isaisabel August 31, 2021 18:23
Copy link
Contributor

@isaisabel isaisabel left a comment

Choose a reason for hiding this comment

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

Summary: fantastic start, all the required changes are there but it needs a bit of cleanup.

Toolbar section headers

When the tabs are hidden, the toolbar section headers are as well. This is fine. However, the sleight overlap with the toolbar creates a white pixel within the toolbar, which looks strange. Removing the white border or the overlap should rectify this issue. The white border is for visibility when many layers are open simultaneously.
Screen Shot 2021-08-31 at 2 36 30 PM
Screen Shot 2021-08-31 at 2 36 36 PM

Also interacts strangely with tabs when window is narrow or many tabs are present:
image

Create layer from other layers compatability

  1. Create layer from other layers marks tabs with a variable name for use in the score input. The variable marker is no longer aligned readibly due to the padding removal:
    Screen Shot 2021-08-31 at 2 38 52 PM
  2. The user can scroll in that interface, hiding the layer variable marker from view when the tabs hide. It may be worth only allowing the tabs to be hidden on scroll when on a layer tab. Alternatively, move the layer variables from the tabs themselves into a list adjacent to the input (would require documentation updates)
  3. Image in help page of layer variables probably needs to be recreated with new UI:

Screen Shot 2021-08-31 at 2 44 30 PM

Bug with changing tabs

If the user scrolls partway down the page and then closes the last tab or switches tabs otherwise, the tab bar may be hidden even though the scroll is now at the top of the screen. This can make the tabs bar inaccessible. To fix this, recompute the amount of hidden tab every time a layer is changed.
image

Active tab color and style

  1. Active tab doesn't need the bottom border color (a separate element of class and type mat-ink-bar) since it's already denoted by background color:

image

  1. Tab is actually a different color than the toolbar. It should be the same color. This is because mat-tab-link has an opacity of 0.6 when not focused. Removing this will improve the UI:

image

Invisible help button

For some reason there is an invisible help button to the right of the new tab button.
Screen Shot 2021-08-31 at 2 57 38 PM

Seems to be a white help button that is usually invisible, can be seen when banner is present:

Screen.Recording.2021-08-31.at.3.03.01.PM.mov

Closing a tab enables the accessibility scroll arrows

Closing a layer tab for some reason enables the scroll arrows even though there's nothing to scroll to. Opening a new tab removes them.
image

Banner issues

Banner interacts strangely with tabs, and the help button in the banner is not properly aligned.
image

Banner should scroll out of view alongside tabs, but currently only the tabs area scrolls when the user scrolls.

nav-app/src/app/tabs/tabs.component.html Outdated Show resolved Hide resolved
nav-app/src/app/tabs/tabs.component.html Outdated Show resolved Hide resolved
nav-app/src/app/tabs/tabs.component.html Outdated Show resolved Hide resolved
nav-app/src/app/tabs/tabs.component.html Outdated Show resolved Hide resolved
nav-app/src/app/tabs/tabs.component.html Outdated Show resolved Hide resolved
nav-app/src/app/tabs/tabs.component.html Show resolved Hide resolved
nav-app/src/app/tabs/tabs.component.scss Outdated Show resolved Hide resolved
nav-app/src/app/datatable/data-table.component.ts Outdated Show resolved Hide resolved
@iguannalin iguannalin requested a review from isaisabel September 2, 2021 13:50
Copy link
Contributor

@isaisabel isaisabel left a comment

Choose a reason for hiding this comment

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

Looks great! I have found a few bugs and have one design concern.

  1. the floating help button covers some of the layer controls when scrolled. Maybe the help button shouldn't be floating, and instead scroll out of view when the tabs hide?

    image

  2. When the banner is present, the user can create multiple scrollable areas if the viewport is short, which is a bit strange. Depending on mouse position different areas scroll which is very strange to work with:
    image
    image

nav-app/src/app/datatable/data-table.component.ts Outdated Show resolved Hide resolved
@iguannalin iguannalin requested a review from isaisabel September 7, 2021 13:50
…gator into feature/#340-ui-scroll-refactor

# Conflicts:
#	nav-app/src/app/app.module.ts
#	nav-app/src/app/datatable/data-table.component.html
#	nav-app/src/app/datatable/data-table.component.ts
@iguannalin iguannalin merged commit 4bbcaaf into develop Sep 7, 2021
@clemiller clemiller deleted the feature/#340-ui-scroll-refactor branch February 17, 2022 16:47
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.

UI Scrolling and whitespace refactor
2 participants