-
Notifications
You must be signed in to change notification settings - Fork 187
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
BottomBar replaced with BottomToolBar #4351
Conversation
Hi @MisRob, |
Thanks @cerdo03, I asked my colleagues for review.
If it's not used from anywhere (which I assume isn't since that's the main task here?) then yes, please |
@@ -250,7 +250,7 @@ | |||
import Diff from './Diff'; | |||
import { fileSizeMixin, titleMixin, routerMixin } from 'shared/mixins'; | |||
import { ContentKindsNames } from 'shared/leUtils/ContentKinds'; | |||
import BottomBar from 'shared/views/BottomBar'; | |||
import BottomToolBar from '../../../shared/views/BottomToolBar.vue'; |
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.
Thanks @cerdo03 , The changes looks good to me, after the deleting BottomBar.vue file
we should be able to merge it.
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.
@AllanOXDi I have deleted the BottomBar.vue file. Please review if this is ready to merge.
Hi @cerdo03 - thank you so much for your work here! We are so glad to have your contributions to Studio. Before merge, please see if you can resolve your linting errors. You can use Once this is complete, will be happy to accept your contribution as it is, but I also wanted to share with you that your PR prompted a small discussion within our team about these two files. In the future, we will begin moving away from using Vuetify, which is the component framework you see in some files where a component is pre-pended with "V", such as Since it was not described in the original issue, we will gladly accept your contribution, and I can create a follow-up task for someone on our team to complete. But, if you would be interested in continuing to work on this, switching the solution by replacing Thank you again for your interest in contributing to Studio! |
@marcellamaki I would love to continue working on this issue. I'll make the desired changes and raise an updated PR soon. Thank you for the feedback. |
Looks like this work is being finalized in #4359 - so closing this PR in favour of that one. |
Summary
Description of the change(s) you made
Fixes #2008
I have replaced all occurences of BottomBar with BottomToolBar. Modified the import statements accordingly. Should I delete the BottomBar.vue file? Please review and let me know if my approach is right or do I need to take care of anything else?
Reviewer guidance
How can a reviewer test these changes?
Should I delete the BottomBar.vue file?
References
Comments
Contributor's Checklist
PR process:
CHANGELOG
label been added to this PR. Note: items with this label will be added to the CHANGELOG at a later timedocs
label has been added if this introduces a change that needs to be updated in the user docs?requirements.txt
files also included in this PRStudio-specifc:
notranslate
class been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)pages
,components
, andlayouts
directories as described in the docsTesting:
Reviewer's Checklist
This section is for reviewers to fill out.
yarn
andpip
)