-
Notifications
You must be signed in to change notification settings - Fork 315
refactoring #1343
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
refactoring #1343
Conversation
|
Thank you, more changes are useful. But there is a bit of case I'd like to keep the original struct name to more clearly. |
|
|
||
| fn open_drawer_at<F>(&mut self, placement: Placement, cx: &mut App, build: F) | ||
| where | ||
| F: Fn(Drawer, &mut Window, &mut App) -> Drawer + 'static, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
&mut Window, &mut App is a standard in entire of the GPUI application, so we can keep the original style.
| F: FnOnce(&mut Self, &mut Window, &mut Context<Self>) -> R, | ||
| { | ||
| let root = window | ||
| .root::<Root>() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, I'd like to use Root, not Self.
| } | ||
|
|
||
| /// Check if the self is equal to the given index path (Same section and row). | ||
| pub fn eq_row(&self, index: IndexPath) -> bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As an argument, I'd like to keep original style.
| AlertVariant::Info => cx.theme().info, | ||
| AlertVariant::Success => cx.theme().success, | ||
| AlertVariant::Warning => cx.theme().warning, | ||
| AlertVariant::Error => cx.theme().danger, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good, now code is shorter.
|
|
||
| impl ThemeColor { | ||
| /// Create a new `ThemeColor` from a `ThemeConfig`. | ||
| pub(crate) fn apply_config(&mut self, config: &ThemeConfig, default_theme: &ThemeColor) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert this.
| let mode = mode.into(); | ||
| if !cx.has_global::<Theme>() { | ||
| let mut theme = Theme::default(); | ||
| if !cx.has_global::<Self>() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Downcast or like this, I'd like revert.
|
|
||
| impl Node { | ||
| fn render_list_item( | ||
| item: &Node, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert this.
| } | ||
|
|
||
| /// Add a ResizablePanelGroup as a child to the group. | ||
| pub fn group(self, group: ResizablePanelGroup) -> Self { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert this.
| self | ||
| } | ||
|
|
||
| pub(crate) fn active_submenu(&self) -> Option<Entity<PopupMenu>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert this.
| } | ||
| } | ||
|
|
||
| pub fn add_child(&mut self, panel: PanelState) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert this.
| if let Some(view) = panel { | ||
| if let Ok(tab_panel) = view.view().downcast::<TabPanel>() { | ||
| Some(tab_panel) | ||
| } else if let Ok(stack_panel) = view.view().downcast::<StackPanel>() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert this.
| if let Some(view) = first_panel { | ||
| if let Ok(tab_panel) = view.view().downcast::<TabPanel>() { | ||
| Some(tab_panel) | ||
| } else if let Ok(stack_panel) = view.view().downcast::<StackPanel>() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert this.
| pub(super) fn replace_panel( | ||
| &mut self, | ||
| old_panel: Arc<dyn PanelView>, | ||
| new_panel: Entity<StackPanel>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert this.
| fn assert_panel_is_valid(&self, panel: &Arc<dyn PanelView>) { | ||
| assert!( | ||
| panel.view().downcast::<TabPanel>().is_ok() | ||
| || panel.view().downcast::<StackPanel>().is_ok(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert this.
| /// Initialize the panel registry. | ||
| pub(crate) fn init(cx: &mut App) { | ||
| if let None = cx.try_global::<PanelRegistry>() { | ||
| cx.set_global(PanelRegistry::new()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert this.
| /// Set `None` in `sizes` to make the index of panel have auto size. | ||
| pub fn split_with_sizes( | ||
| axis: Axis, | ||
| items: Vec<DockItem>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert this.
|
Since this PR has received no feedback for a long time and code conflicts, I've temporarily closed it. If you wish to continue, you can reopen it and refine it. |
No description provided.