Skip to content

WIP: Resolve "Collapsable sidebar in the web IDE"

Chad Woolley requested to merge 22457-collapsable-sidebar-in-the-web-ide into master

NOTE: This MR has been closed without merging, in favor of a more incremental approach which is contained in this epic.

NOTE: Part of this work was split out to a separate preliminary refactoring which has been completed.

What does this MR do?

Screenshots

Task List

  • Refactor out a generic CollapsibleSidebar component which can be reused with a a new leftPane vuex namespaced module.
  • Handle loading state from ide_side_bar.vue (move to collapsible_sidebar.vue). Note it now behaves slightly differently, it is only shown when there is an open tab, not during the whole loading phase like before.
  • Using currentActivityView via UPDATE_ACTIVITY_BAR_VIEW mutation to set leftPane.currentView
  • In CollapsibleSidebar, hook "open/close" callbacks to sync with currentActivityView when used on left side. Best way may be to just to conditionally call the mutation in open action.
  • Show project icon and header.
    • Finish UI and styling for project icon and header under new DOM structure.
    • Both logo and text should be highlighted on hover (see design feedback comment)
  • Question: How should aliveViews work on left side? Are they all always "alive" (rendered)? Answer: Since ide_side_bar only ever rendered currentActivityView, they should all be keepAlive=false unless I'm missing something.
  • In CollapsibleSidebar, provide a way to pass down custom classes.
  • Replace all occurrences of activityBarViews.key with leftSidebarViews.key.name.
  • Use correct width for left sidebar (340px) vs right (350px).
  • BUG: Look into repo_editor.vue error found while exploring locally.
  • BUG: look into multiple copies of tab components showing up
  • For new empty-repo projects, should open Edit pane by default (otherwise many feature specs fail because they assume the ide-tree is visible).
  • Switch to new v-slot syntax instead of deprecated slot syntax
  • Left sidebar Commit button/tab should only appear if hasChanges is true.
  • After clicking "Stage & Commit" from commit panel, Edit panel should be shown with message "Your changes have been committed"
  • "Discard Changes" (trash can icon) button at top of expanded commit panel "Unstaged/Staged Changes" view should still work, and reset view to expanded Edit view when changes are discarded.
  • "Discard Changes" button at top right of commit view should still work, and reset view to expanded Edit view when all changes are discarded. (started working after rebase to master)
  • Backfill/cleanup tests
    • New test file for left.vue
    • Check spec files for ide_side_bar.vue and activity_bar.vue for tests to migrate.
    • Add tests for uncovered logic in collapsible_sidebar.vue
    • add some new coverage to integration test spec/features/ide_spec.rb Maybe not necessary; there is some more coverage in the QA test suites under qa/qa/specs/features.
  • After things are functional with new approach, remove usages of currentActivityView and updateActivityBarView. Refactor them away and just use leftPane.currentView and leftPane.open, because they are now duplicate and it would require a hack to keep them in sync.
  • BUG: Making a change and committing to master tries to use incorrect branch instead of master, e.g.: http://0.0.0.0:3000/-/ide/project/root/test-project-1/blob/root-master-patch-16453/-/README.md
  • Use createNamespacedHelpers instead of doing it manually.
  • Doublecheck for missed logic in ide_side_bar.vue and activity_bar.vue
  • Delete ide_side_bar.vue and activity_bar.vue and their specs, check for unused classes/methods to delete.
    • Delete activityBarViews from ide/constants.js
    • Remove/Update references from qa/qa/page/project/web_ide/edit.rb page objects
    • Ensure all QA specs specs still pass.
  • Understand why left-side components (e.g. commit_sidebar/form.vue and its child commit_sidebar/actions.vue) are now re-mounted (when they didn't used to be) even though they are hard-coded in ide/components/panes/left.vue (asked on internal slack). ANSWER It was because multiple resizable-panels were introduced, and fixed by going back to one. See this note for details.
  • Compact mode is no longer properly styled when window is short (shorter than MAX_WINDOW_HEIGHT_COMPACT of 750px). It is too big and leaves buttons off the bottom of the screen. Need to preserve appropriate styles.
  • Ensure create_directory_spec and create_file_spec pass again; with and without CHROME_HEADLESS=0 (they now do on master, as of !23360 (merged))
  • During QA specs (e.g. create_file_spec), the bottom ide-status-bar is floating in the middle of the screen. This seems to have caused some intermittent QA spec failures. This is reproducible during normal usage by making the window shorter than MAX_WINDOW_HEIGHT_COMPACT, and then refreshing, or changing from commit to edit tabs. See screenshot in this comment.
  • Missing vertical scrollbar for ide_tree (edit pane) when it is shorter than the list of files.

Tasks to pull out to a separate MR

Development Checklist

Copied (partially) from Frontend Guide Development Process Development Checklist

Frontend development

Planning development

  • Check the current set weight of the issue, does it fit your estimate?
  • Are all departments that are needed from your perspective already involved in the issue? (For example is UX missing?)
  • Is the specification complete? Are you missing decisions? How about error handling/defaults/edge cases? Take your time to understand the needed implementation and go through its flow.
  • Are all necessary UX specifications available that you will need in order to implement? Are there new UX components/patterns in the designs? Then contact the UI component team early on. How should error messages or validation be handled?
  • Library usage Use Vuex as soon as you have even a medium state to manage, use Vue router if you need to have different views internally and want to link from the outside. Check what libraries we already have for which occasions.
  • Plan your implementation:
    • Architecture plan: Create a plan aligned with GitLab's architecture, how you are going to do the implementation, for example Vue application setup and its components (through onion skinning), Store structure and data flow, which existing Vue components can you reuse. It's a good idea to go through your plan with another engineer to refine it.
    • Task list: Create a simple checklist of the subtasks that are needed for the implementation, also consider creating even sub issues. (for example show a comment, delete a comment, update a comment, etc.). This helps you and also everyone else following the implementation
  • Keep it small To make it easier for you and also all reviewers try to keep merge requests small and merge into a feature branch if needed. To accomplish that you need to plan that from the start. Different methods are:
    • Skeleton based plan Start with an MR that has the skeleton of the components with placeholder content. In following MRs you can fill the components with interactivity. This also makes it easier to spread out development on multiple people.

During development

  • Check off tasks on your created task list to keep everyone updated on the progress
  • Share your work early with reviewers/maintainers
  • Share your work with UXer and Product Manager with Screenshots and/or GIF's. They are easy to create for you and keep them up to date.
  • If you are blocked on something let everyone on the issue know through a comment.
  • [-] Documentation Update/add docs for the new feature, see docs/. Ping one of the documentation experts/reviewers

Finishing development + Review

  • Keep it in the scope Try to focus on the actual scope and avoid a scope creep during review and keep new things to new issues.
  • Performance Have you checked performance? For example do the same thing with 500 comments instead of 1. Document the tests and possible findings in the MR so a reviewer can directly see it.
  • Have you tested with a variety of our supported browsers? You can use browserstack to be able to access a wide variety of browsers and operating systems.
  • Did you check the mobile view?
  • [-] Check the built webpack bundle (For the report run WEBPACK_REPORT=true gdk run, then open webpack-report/index.html) if we have unnecessary bloat due to wrong references, including libraries multiple times, etc.. If you need help contact the webpack domain expert Doesn't work, asked on internal slack
  • Tests Not only greenfield tests - Test also all bad cases that come to your mind.
  • [-] If you have multiple MR's then also smoke test against the final merge.
  • [-] Are there any big changes on how and especially how frequently we use the API then let production know about it
  • Smoke test of the RC on dev., staging., canary deployments and .com
  • Follow up on issues that came out of the review. Create issues for discovered edge cases that should be covered in future iterations.

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team

Closes #22457 (closed)

Edited by Chad Woolley

Merge request reports