Skip to content

refactor(ControlBar): implement redesigned control bar#165

Open
ShaneHe023 wants to merge 7 commits into
mainfrom
control-bar-new
Open

refactor(ControlBar): implement redesigned control bar#165
ShaneHe023 wants to merge 7 commits into
mainfrom
control-bar-new

Conversation

@ShaneHe023
Copy link
Copy Markdown
Collaborator

@ShaneHe023 ShaneHe023 commented Oct 7, 2023 •
edited by Lutra-Fs
Loading

Close #151

@Lutra-Fs
Copy link
Copy Markdown
Member

Lutra-Fs commented Oct 7, 2023

image

Lutra-Fs
Lutra-Fs reviewed Oct 7, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@Lutra-Fs Lutra-Fs left a comment

Choose a reason for hiding this comment

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

I would say, restore/ save should be moved to another component. since they are not sharing location anymore

Lutra-Fs
Lutra-Fs requested changes Oct 7, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@Lutra-Fs Lutra-Fs left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Styling issue needs to be fixed.
Conflict needs to be resolved.
You are not using standard formatting. run npm run format to fix formatting issues (DO NOT push other files after running the command!)

@ShaneHe023
Copy link
Copy Markdown
Collaborator Author

ShaneHe023 commented Oct 7, 2023

I would say, restore/ save should be moved to another component. since they are not sharing location anymore

is it okay that I just set Ctrl bar above the Save/restore buttons for now?

@LorenzoValentine
Copy link
Copy Markdown
Member

LorenzoValentine commented Oct 7, 2023

I would say, restore/ save should be moved to another component. since they are not sharing location anymore

is it okay that I just set Ctrl bar above the Save/restore buttons for now?

What do you mean by 'Ctrl' bar? the Ctrl key in the keyboard?

@LorenzoValentine
Copy link
Copy Markdown
Member

LorenzoValentine commented Oct 7, 2023

above the Save/restore buttons for now

'above the Save/restore buttons for now' Do you think this is a good idea of fixing? Or you just want to make you fixing period shorter?

@Lutra-Fs Lutra-Fs force-pushed the main branch 2 times, most recently from 89bac3b to 0085d96 Compare October 9, 2023 12:17
@Lutra-Fs
style: run prettier
47a28d4 
Signed-off-by: Bill ZHANG <36790218+Lutra-Fs@users.noreply.github.com>
@Lutra-Fs
fix: merge branch 'main' into control-bar-new
28e4dae 
Signed-off-by: Bill ZHANG <36790218+Lutra-Fs@users.noreply.github.com>
github-advanced-security[bot]
github-advanced-security AI found potential problems Oct 10, 2023
View reviewed changes
Comment thread src/components/ControlBar.tsx Fixed
Comment thread src/components/ControlBar.tsx Fixed
Lutra-Fs and others added 3 commits October 11, 2023 10:55
@Lutra-Fs
fix(ControlBar): fix eslint error
205d68e 
Signed-off-by: Bill ZHANG <36790218+Lutra-Fs@users.noreply.github.com>
@Lutra-Fs
refactor(ControlBar): remove not used export
6c1f8d9 
Signed-off-by: Bill ZHANG <36790218+Lutra-Fs@users.noreply.github.com>
fix(controlBar): fix button overlapping
cd355ea
@Lutra-Fs Lutra-Fs self-requested a review October 28, 2023 17:21
Lutra-Fs
Lutra-Fs requested changes Nov 13, 2023
View reviewed changes
Comment thread src/components/ControlBar.tsx
`;

const ControlBarBtn = styled(Button)`
shape: circle;
Copy link
Copy Markdown
Member

@Lutra-Fs Lutra-Fs Nov 13, 2023

Choose a reason for hiding this comment

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

shape is not in CSS properties.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Lutra-Fs Lutra-Fs Lutra-Fs requested changes

@LorenzoValentine LorenzoValentine Awaiting requested review from LorenzoValentine

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

implement control bar redesigns

4 participants

@ShaneHe023 @Lutra-Fs @LorenzoValentine @github-advanced-security