Divide card content into left and right containers

lpandath commented 1 month ago

Description

This PR introduces an instruction toggle with buttons located on the left side of the interface, allowing users to navigate between the primary and secondary views of the content. The left side includes the step-by-step navigation, while the right side dynamically displays the relevant content based on the selected step and view.

What does this PR do?

Breaking changes

Code review focus

Dependencies

Checks

[ ] Summary updated
[ ] Version view updated
[ ] Documentation added
[ ] Tests added
[ ] E2e tests created
[ ] Successfully ran e2e tests before merge

closes GH-

ValentinFutterer commented 1 month ago

It looks very good, i have just a few small general things i would like to add:

To switch between the views, i would suggest to use mat-tab-group instead of using buttons.

I could also see the use in a button on the end of each component, that jumps to the next stepper. If one component gets very long, the user would have to scroll up. Or in the case of step 9, down.

Maybe it would improve the look if the stepper and content component would always have the same size, so that they end at the same point. Could just be me though :)

On smaller screens, content gets cut, off, e.g.g this happens on my work laptop, which is relatively big: On even smaller screens and befoire the switch to mobile, the cutoff gets progressively worse. This should be addressed somehow. Maybe by making the stepper component slimmer and adding horizontal scrolling to the main content component and automatically close the sidebar to save on space.

On mobile, the cutoff also happens, but since we dont try to support mobile perfectly i think thats fine for now, as long as users can still scroll around on touchscreen. I cant confirm that though. If they cannot, then its basically unusable on mobile, since even the action bar is cut off.

lpandath commented 1 month ago

General This is a huge feature. Thank you for digging into this! Observed issues are listed below.

Moving forward, maybe it makes sense to split this feature into smaller chunks (basically as you already did with separate commits)

* have a feature branch (this one)

* for each step

  * create an issue
  * align on the design

    * which options should be on the left side?
    * what should be possible on the right side?
    * general information that should be displayed on the right side for either option (e.g. project for step one)

* work on each step/issue separately

* create PR for feature branch

When all separate issues for the split view are fixed and merged, commits of the feature branch can be squashed (step is working as expected). Rebase onto next if required (probably a good idea to keep up-to-date with bug fixes and changes) Create a PR (or keep this one open as a draft), merging the feature branch into next.

Even though I am not a huge fan of long lasting feature branches, this is probably a valid approach here. And a change like this should only get in if we are certain everything is still usable as it was before.

Otherwise unexpected layout behaviour or illogical UX might get into the main branch. When doing a release in-between, the commits would have to be removed manually - which is quite tedious with larger changes.

Issues Components have been implemented to fit the previous design. Some behave in an illogical way now or do not fit the new layout. This is probably expected when changing the overall layout without adapting each component. As explained before, getting this feature in should ensure that everything is usable in a proper way.

1. Probably makes sense to have the stepper fixed/sticky in order to be always visible
   ![image](https://private-user-images.githubusercontent.com/72449192/375461275-53e3fd90-aae4-4468-beb1-d8db364ed58c.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mjg1ODAwOTcsIm5iZiI6MTcyODU3OTc5NywicGF0aCI6Ii83MjQ0OTE5Mi8zNzU0NjEyNzUtNTNlM2ZkOTAtYWFlNC00NDY4LWJlYjEtZDhkYjM2NGVkNThjLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDEwMTAlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMDEwVDE3MDMxN1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPThmMzk3YjhmODJkNWNjMDU2NmIxMDk3ZDMwZThiMGMyOWQzZWI0MTYwY2JlYWY0ZjFhNTk2ZWZiYTE4ZGFlOWQmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.36tBJu_pI7nckpTtYiOrIX0ZxMxKKgHlQxws70dgCMY)

2. Have a fixed/sticky footer
   ![image](https://private-user-images.githubusercontent.com/72449192/375461779-de11e12b-cc0a-4fdf-b345-3ab8eefb3318.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mjg1ODAwOTcsIm5iZiI6MTcyODU3OTc5NywicGF0aCI6Ii83MjQ0OTE5Mi8zNzU0NjE3NzktZGUxMWUxMmItY2MwYS00ZmRmLWIzNDUtM2FiOGVlZmIzMzE4LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDEwMTAlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMDEwVDE3MDMxN1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTE1MzViMjU2NzAxNDAyNTc5ZGJkZGMwYzg1YjA2OGM3MGRlM2MwM2UxMDA2MTY3N2FiMWViZmUzOGQ1MmQ0NGQmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0._YefOsJVV-fxSwqrkgvyY1EEQb6hVFngMmNOyf_SY48)

3. In step 3, the right content will wrap depending on its size when selecting an option on the left side (example width of 1120px)
   ![image](https://private-user-images.githubusercontent.com/72449192/375462373-64698456-2dcc-4d70-bf91-f960bbf8f974.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mjg1ODAwOTcsIm5iZiI6MTcyODU3OTc5NywicGF0aCI6Ii83MjQ0OTE5Mi8zNzU0NjIzNzMtNjQ2OTg0NTYtMmRjYy00ZDcwLWJmOTEtZjk2MGJiZjhmOTc0LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDEwMTAlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMDEwVDE3MDMxN1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWMxNDI3ZTA0MjM0NDZhYmFhNWJjMjY1ZmY5Y2Q0NDNmMTA0YWQ5YzAyNzE1NmFkOThhZjE4MWQ5OTZhYTBhYmImWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.V30Nx-8olkOMVhxUtnIrqCNdTUO7jI4DTUP7_Dd6BoQ)

4. In step 3, overflowing content will be hidden without a possibility to scroll (1300px < width < 1360px). It is fine when the menu is closed.
   ![image](https://private-user-images.githubusercontent.com/72449192/375463772-02ff3959-1e31-4c90-826d-e48562eac757.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mjg1ODAwOTcsIm5iZiI6MTcyODU3OTc5NywicGF0aCI6Ii83MjQ0OTE5Mi8zNzU0NjM3NzItMDJmZjM5NTktMWUzMS00YzkwLTgyNmQtZTQ4NTYyZWFjNzU3LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDEwMTAlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMDEwVDE3MDMxN1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWNjNzBmMDZjZjQyZjc2MDZhNjU4NjhmMzMyYmE1MWVmMjE3ZjVjMWVkMWJhNzk5ODlhMmFiZTdjMWMxNmU2NmUmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.BV393giZHqGCsu8GZtBN6WRs117f8-7S38XTTakGB60)
   ![image](https://private-user-images.githubusercontent.com/72449192/375463882-588dc1d3-fc64-43e7-a915-648774b88dcc.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mjg1ODAwOTcsIm5iZiI6MTcyODU3OTc5NywicGF0aCI6Ii83MjQ0OTE5Mi8zNzU0NjM4ODItNTg4ZGMxZDMtZmM2NC00M2U3LWE5MTUtNjQ4Nzc0Yjg4ZGNjLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDEwMTAlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMDEwVDE3MDMxN1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWQ2NDNiMTAxZTczMTBkMzYzMTNiNDAyMDA3ZWQ4OGFhNjJiM2E4NWExY2MzZGM1N2Y1MGQxNmIyYTE5MmMwNWUmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.xR8HmPFhClXH_e2yaFRiPGo6wzCLhwmiyJRpZ58hmSA)

5. Is the gap between steps on purpose when there are no options for the left side? Maybe it makes sense to highlight the selected step some other way? Maybe use an outline icon style for other steps than the current one? This was probably not an issue before as it was clear which step a user is currently on.
   ![image](https://private-user-images.githubusercontent.com/72449192/375465290-7e0535ca-bdb3-4aa7-9f8e-e8c09035db2a.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mjg1ODAwOTcsIm5iZiI6MTcyODU3OTc5NywicGF0aCI6Ii83MjQ0OTE5Mi8zNzU0NjUyOTAtN2UwNTM1Y2EtYmRiMy00YWE3LTlmOGUtZThjMDkwMzVkYjJhLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDEwMTAlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMDEwVDE3MDMxN1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTMzMmVmMzJhOGI0Y2I5MGRmZDgyYjlmMGI4MWMwNjhlNjM2ZDBjYTk3NTljMWUwMzU4N2E4N2RlMjUyMmJlMjAmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.sBJV-sEpHpvr1EPBmKZV8ekIaNsxU5t8Sr8p61bDBJM)

6. Step 5 has cut off content
   ![image](https://private-user-images.githubusercontent.com/72449192/375471888-e7d93c41-e2e1-49d4-bd43-30bb56fd2bad.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mjg1ODAwOTcsIm5iZiI6MTcyODU3OTc5NywicGF0aCI6Ii83MjQ0OTE5Mi8zNzU0NzE4ODgtZTdkOTNjNDEtZTJlMS00OWQ0LWJkNDMtMzBiYjU2ZmQyYmFkLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDEwMTAlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMDEwVDE3MDMxN1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTVjMDY0NTlhMGU3MTAwNmU2N2EzNTQ3M2UwY2ZkNjQzNzBlMzU5MDliMzY5YTQwNDZjZjVkNTc3MTM3YWE5YWEmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.3Pxvj5gUwv1CUYlopPvy7KWkmX7A1gtAT4jKnHB7n4c)

7. Step 7. Marking a dataset as deleted wraps the element
   ![image](https://private-user-images.githubusercontent.com/72449192/375471351-ddb248a2-7a62-453b-a84d-3cb1045a1336.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mjg1ODAwOTcsIm5iZiI6MTcyODU3OTc5NywicGF0aCI6Ii83MjQ0OTE5Mi8zNzU0NzEzNTEtZGRiMjQ4YTItN2E2Mi00NTNiLWE4NGQtM2NiMTA0NWExMzM2LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDEwMTAlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMDEwVDE3MDMxN1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTY3MWM3NjY4YTFlYjNhMWMwYTlhNmJhYjZkNWU2NTBhNjZiMWUxOGNlYmIwNjllZWQzNDBiMmViNWUwMmMyMjkmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.14siRo7RgTwO49l9-6yblMeLqgoE2fjPLaKiDxsk5Uc)
   ![image](https://private-user-images.githubusercontent.com/72449192/375471431-d7e4a943-d2aa-4703-8dd1-4034a737ffa0.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mjg1ODAwOTcsIm5iZiI6MTcyODU3OTc5NywicGF0aCI6Ii83MjQ0OTE5Mi8zNzU0NzE0MzEtZDdlNGE5NDMtZDJhYS00NzAzLThkZDEtNDAzNGE3MzdmZmEwLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDEwMTAlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMDEwVDE3MDMxN1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPThiMjY3OTRlMzdhZjJlYjQwZjQwN2MyZmM5MWIwMjg5YzI2YTI3YzY3ZDkzZmJmZGQxNTE3MmYzYTM1NjRiOTAmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.sENjo1JVYB4cTEbhBPt45zH-zzaHA1VUgq527Q_33tY)

8. On smaller screens, the vertical stepper takes up quite some space and it becomes tedious having to scroll up and down. This happens when trying to change the option for the same step or when trying to get to the next step every time. Maybe it makes sense to have `next`/`previous` buttons somewhere? Or have the stepper horizontally instead of vertically for smaller screens?

(Screenshots from a width of 930px which is probably not that small after all)

@rekt-hard Thank you so much for reviewing the PR. Yes, this feature is indeed huge! I based the implementation on the initial XD examples, but as you mentioned, it makes sense now to go deeper into each step after all the changes. The detailed views for each step were something we agreed to tackle later, but now I see the need to address them.

@DererekMolnar, I think we should have a discussion tomorrow on how we want to proceed with these detailed views, as there haven’t been any suggestions so far. We can discuss it tomorrow for the initial plan for the next steps.

ValentinFutterer commented 1 month ago

On smaller screens, the vertical stepper takes up quite some space and it becomes tedious having to scroll up and down. This happens when trying to change the option for the same step or when trying to get to the next step every time. Maybe it makes sense to have next/previous buttons somewhere? Or have the stepper horizontally instead of vertically for smaller screens?

The next button would be a great idea!

I have experimented today with a horizontal stepper, but i did not have much success, the stepper component doesnt offer a way to only show parts of it. So for e.g. i am at step 4 and i see 4, 5, 6, but i can navigate forwards or backwards with little arrows. Although there is a hacky way to maybe get this working, i havent tried this yet though.

I have also experimented with making the stepper component itself smaller, and i am very confident that i can do that, this could save ~130px on space, depending on the exact values chosen.

There is also the option of making everything inside the components itself smaller - which would be a very tedious process, but effective. This work has to be done anyways to some extent, since the old css @media queries relied on there being more space for them, so now they trigger at unwanted times.

Lastly, i would suggest to trigger the auto-minimize of the sidebar way earlier, at around 1300px. I am aware this will effect the whole application, but i think its worth it.

andresTabiTuwien commented 1 week ago

I did the review and I found that in general everything is working well. I found some things that I think need to be checked, I'm not sure if they should be fixed in this PR or we should create another small issue to fix them.

Here is the list of things I found.

In step 1 in screens smaller than 1680 the end date of the calendar does not appear and in screens smaller than 1300 it appears behind the text of the project description.
In step 3, the table and text area are cropped in screens smaller than 1000px when new data is tapped.
In step 5, in the Storage tab, the fields and the text area are cut into screens smaller than 900px.

4.At step 8, on the Repository Search tab, the buttons for navigating the pagination are half visible. step8

In step 10, if we have a list of costs and one of them is open, we will not be able to see the other cost items.
In some steps we are getting this errror in console: "ERROR RuntimeError: NG0100: ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous value: 'undefined'. Current value: '[object Object]'." We can try to identify what generates the error and implement a strategy to avoid it. This coulb be as a new issue. Steps that generate the error are 1, 2, 6, 8.

lpandath commented 1 week ago

Thank you for the review, @andresTabiTuwien! 👍

Here’s a summary of the changes made:

Step 1: Fixed display issues with the end date in the calendar. On screens smaller than 1680px, the end date now appears correctly, and for screens below 1300px, it no longer overlaps with the project description text.
Step 5 (Storage Tab): Adjusted layout so fields and the text area display correctly on screens smaller than 900px.
Step 8 (Repository Search Tab): Pagination navigation buttons are now fully visible.
Step 10: Fixed scrolling within the list of costs to ensure all items are accessible when one is expanded.
Entire Stepper: Added padding/margin adjustments to ensure step 11 is fully visible.

Dataset Table on Step 3: I couldn’t reproduce the issue fully, as the component needs a complete refactor. I tested it without the table, and it worked as expected based on the screenshot.

Console Error: ExpressionChangedAfterItHasBeenCheckedError in steps 1, 2, 6, and 8 also resolved.

If everything looks good, I’ll go ahead squash the commits and merge.

andresTabiTuwien commented 1 week ago

Hi @lpandath, I have checked the points and they are all solved. Everything works really well.

lpandath commented 1 week ago

The PR has been merged. Due to internal conflicts, I had to merge it directly. I thoroughly tested it multiple times beforehand to ensure there are no issues.