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

MAUI Blazor sidebar and fluent ui base design #791

Merged
merged 7 commits into from
Apr 18, 2023
Merged

Conversation

btiteux
Copy link
Collaborator

@btiteux btiteux commented Apr 7, 2023

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • New feature or enhancement
  • UI change (please include screenshot!)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Internationalization and localization
  • Other (please describe):

What is the current behavior?

Devtoy 2.0 doesn't have blazor component and fluent design

Issue Number: N/A

What is the new behavior?

Added a blazor Navbar component for .net MAUI.

Other information

image
image

Remaining TODO:

  • Use Event from ViewModel.
  • Some CSS adjustment
  • Search form

Quality check

Before creating this PR:

  • Did you follow the code style guideline as described in CONTRIBUTING.md
  • Did you build the app and test your changes?
  • Did you check for accessibility? On Windows, you can use Accessibility Insights for this.
  • Did you verify that the change work in Release build configuration
  • Did you verify that all unit tests pass
  • If necessary and if possible, did you verify your changes on:
    • Windows
    • macOS (DevToys 2.0)
    • Linux (DevToys 2.0)

@btiteux btiteux requested a review from veler April 7, 2023 15:37
Copy link
Collaborator

Choose a reason for hiding this comment

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

The WASDK app also use this TTF. Could we move the TTF, WOFF, WOFF2 to a shared folder to avoid having copies in the repository? This would also ensure we use the same font everywhere.

.vscode/launch.json Outdated Show resolved Hide resolved
.vscode/tasks.json Outdated Show resolved Hide resolved
href = Convert.ToString(obj, CultureInfo.InvariantCulture);
}

_hrefAbsolute = href == null ? null : NavigationManager.ToAbsoluteUri(href).AbsoluteUri;
Copy link
Collaborator

Choose a reason for hiding this comment

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

You set _hrefAbsolute to null but _hrefAbsolute isn't marked as nullable. Should this be string.Empty instead?


<main class="page">
@Body
Copy link
Collaborator

@veler veler Apr 9, 2023

Choose a reason for hiding this comment

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

I'm playing a bit with the code to try to make the components for the tools. I notice that for now we only have Index and it uses the URI to determine the tool name to display.

Thinking about it, I thought OK but we don't have only tools, sometimes we also display a group of tools (like when we select "All Tools") in this area, which isn't a tool itself. I dislike the idea of having the UI for tools and UI for groups in the same page (Index.razor), so I naively I started making a page ToolPage.razor and ToolGroupPage.razor, exactly like in the WASDK app. One page would have the URI tool/{Name} and the other group/{Name} but then I quickly realized that you did it this way because when clicking on a menu item, the whole page would refresh when navigating to a different URL, which is a problem indeed....
In addition of this, I feel like passing the {Name} to the page and using only this to determine what the hell we need to display in the page is a little bit of a pain to maintain (first sign that it's a future pain to maintain is that you have to URL encode/decode it...). We better pass the whole GuiToolViewItem or GroupViewItem instead of just its internal name if possible.

So how about the following:

Instead of navigating to a page, we listen to a MainWindowViewModel.SelectedMenuItem (which I know you will do in a future PR) and when it change, depending on whether it's a GuiToolViewItem or GroupViewItem, we show here a ToolPage component or a ToolGroupPage component instead of @Body (or leave Index totally empty). This way we never navigate anywhere and just replace the body on the fly. In addition, this way we can pass the whole GuiToolViewItem to the ToolPage as a component parameter instead of relying on the internal name to find which tool / group we're trying to display.

Would this work? Would this have any performance impact compared to navigating (which, if I understood well, isn't really navigating at the moment since we don't refresh the whole page, right?) ?
What do you think? :)

Copy link
Collaborator

@veler veler Apr 9, 2023

Choose a reason for hiding this comment

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

PS: This could be done in a different PR by the way. Happy to do it myself too if you want, since I kind of need a minimal navigation working in order to show the tool's UI.

@veler veler merged commit 9d7b8e2 into dev/2.0 Apr 18, 2023
@veler veler deleted the dev/blazor/navbar branch April 18, 2023 22:13
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.

2 participants