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

style fixes and add header and sidebar bg token #2157

Merged
merged 21 commits into from
Oct 27, 2022

Conversation

imanjra
Copy link
Contributor

@imanjra imanjra commented Oct 12, 2022

What changes are proposed in this pull request?

This PR continuous theming refresh PR #2156 in ways listed below:

  • Improve background customization of header
  • Improve background customization of sidebar
  • Fixes some style issues with sidebar
  • TBD

How is this patch tested? If it is not, please explain why.

[WIP] As the changes in this PR are related to the UI of the app, it will be tested visually

Release Notes

Is this a user-facing change that should be mentioned in the release notes?

  • No. You can skip the rest of this section.
  • Yes. Give a description of this change to be included in the release
    notes for FiftyOne users.

(Details in 1-2 sentences. You can just refer to another PR with a description
if this PR is part of a larger change.)

Several color and layout style of the application are being updated. Additionally, a light theme is being added along with a toggle (in the top right) to switch between dark and light theme.

What areas of FiftyOne does this PR affect?

  • App: FiftyOne application changes
  • Build: Build and test infrastructure changes
  • Core: Core fiftyone Python library changes
  • Documentation: FiftyOne documentation changes
  • Other

@imanjra imanjra changed the title style fixes and add header and sidebar bg token [WIP] style fixes and add header and sidebar bg token Oct 12, 2022
@imanjra imanjra marked this pull request as ready for review October 14, 2022 15:49
@imanjra imanjra added app Issues related to App features design Work related to the design of a feature/UI element labels Oct 14, 2022
@imanjra
Copy link
Contributor Author

imanjra commented Oct 17, 2022

Dark mode:
image

Light mode:
image

@brimoor
Copy link
Contributor

brimoor commented Oct 17, 2022

@imanjra nice work! 🥇

Here's my reactions:

General

  • Are you open/able to revert to the rounded components for the OSS App's actions menu (gear, tag, etc) and sidebar filters (N active filters, N fields checkmarks)? I think I prefer them and it doesn't bother me that they have different shape than some other
  • From the light theme screenshot, the font size for the tooltip of the actions menu looks much bigger than other text such as text in filters sidebar (the second, darker tooltip from Flashlight is smaller than sidebar text, but that's not important to fix right now as that's different code)
  • Don't use orange border for Have a team? button. Just use the default border color for the theme (my reasoning here is that it should help make light theme button look better when the background is lighter, per suggestion below)
  • What about putting the light/dark theme toggle to the right of Have a team? button, for consistency with other actions?

Light theme

  • For buttons such as + add stage and Have a team?, I think a lighter (even pure white?) background for these buttons would look better
  • Increase the font weight for main headings like Voxel51 > quickstart? It looks too thin in light mode (but not in dark mode) compared to how Figma design looks (likely due to font quirks, but nonetheless)
  • Minor point: the sidebar looks pretty gray right now. Not sure exactly what to suggest, but perhaps the gray background used for fields like ground_truth could be made lighter?

Copy link
Contributor

@benjaminpkane benjaminpkane left a comment

Choose a reason for hiding this comment

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

LGTM. Made a couple small tweaks.

@benjaminpkane benjaminpkane changed the title [WIP] style fixes and add header and sidebar bg token style fixes and add header and sidebar bg token Oct 27, 2022
@benjaminpkane benjaminpkane merged commit fc93621 into feature/theming-ben Oct 27, 2022
@benjaminpkane benjaminpkane deleted the feature/theming-ben-im branch October 27, 2022 03:16
benjaminpkane added a commit that referenced this pull request Oct 27, 2022
* work

* initial theme pass

* tweaks

* edits

* bugs

* theme tweaks

* fix

* fix

* tweaks

* wrap dataset container with ThemeContext

when a theme prop is provided

* prevent passing value prop to Fragment

* wrap test app with ThemeProvider

* fix view bar crash

* import updates

* mui theme

* update yarn

* query typo

* case change

* proposal

* style fixes and add header and sidebar bg token (#2157)

* style fixes and add header and sidebar bg token

* cleanup theme provider

* update theme override support

* fix font family issue

* fix generated query for path

* light theme fixes first pass

* add theme toggle button

* use app theme for default map theme

* theme and style fixes first pass

* theme mode and compact layout prop support

* fix button tooltip font size issue

* use lighter dynamic color as bg for active filter

* light theme fixes second pass

* use svg element instead of img for looker controls

* add toggle to show or hide outer container header

* looker style enhancements first pass

* merge

* cleaning

* fix

* fix query

Co-authored-by: Benjamin Kane <[email protected]>

* update query

* update deps

* fix merge

Co-authored-by: imanjra <[email protected]>
manivoxel51 pushed a commit that referenced this pull request Nov 8, 2022
* work

* initial theme pass

* tweaks

* edits

* bugs

* theme tweaks

* fix

* fix

* tweaks

* wrap dataset container with ThemeContext

when a theme prop is provided

* prevent passing value prop to Fragment

* wrap test app with ThemeProvider

* fix view bar crash

* import updates

* mui theme

* update yarn

* query typo

* case change

* proposal

* style fixes and add header and sidebar bg token (#2157)

* style fixes and add header and sidebar bg token

* cleanup theme provider

* update theme override support

* fix font family issue

* fix generated query for path

* light theme fixes first pass

* add theme toggle button

* use app theme for default map theme

* theme and style fixes first pass

* theme mode and compact layout prop support

* fix button tooltip font size issue

* use lighter dynamic color as bg for active filter

* light theme fixes second pass

* use svg element instead of img for looker controls

* add toggle to show or hide outer container header

* looker style enhancements first pass

* merge

* cleaning

* fix

* fix query

Co-authored-by: Benjamin Kane <[email protected]>

* update query

* update deps

* fix merge

Co-authored-by: imanjra <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app Issues related to App features design Work related to the design of a feature/UI element
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants