Skip to content
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

refactor(ui): add ModularTable #3668

Closed

Conversation

petermakowski
Copy link
Contributor

@petermakowski petermakowski commented Mar 9, 2022

Done

QA

MAAS deployment

To run this branch you will need access to one of the following MAAS deployments:

Running the branch

You can run this branch by:

QA steps

  • Go to MAAS/r/networks?by=fabric, MAAS/r/networks?by=space and make sure all rows are displayed exactly as before

Fixes

Fixes: # .

Launchpad issue

Related Launchpad maas issue in the form lp#number.

Backports

In general, please propose fixes against master rather than release branches (e.g. 2.7), unless the fix is only applicable for that specific release. Please apply backport labels to the PR (e.g. "Backport 2.7") for the appropriate releases to target.

Only bug and security fixes should be backported, new features should only land in master.

@petermakowski petermakowski force-pushed the add-modular-table branch 3 times, most recently from ad43379 to 7957a4c Compare March 9, 2022 17:36
@petermakowski petermakowski self-assigned this Mar 9, 2022
@petermakowski petermakowski marked this pull request as ready for review March 9, 2022 18:02
@huwshimi
Copy link
Contributor

@petermakowski I know it's a bit awkward working with the external react-components dependency, but I wonder if it might be better to wait for the react-components PR to land and then do this work in a feature branch with a beta release of react-components?

Then when we're happy that all the changes to react-components are made we can release that and then merge the feature branch. What do you think?

Also, not sure if you've spotted this, but maas-ui has a way to link to react-components when developing:

https://github.com/canonical-web-and-design/maas-ui/blob/master/package.json#L32

Which you can use with the scripts in react-components:

https://github.com/canonical-web-and-design/react-components/blob/main/HACKING.md#using-with-another-project

One thing I've found in the past is that you have to run both projects with yarn ... or dotrun ... as using a mix doesn't link the projects correctly.

@petermakowski
Copy link
Contributor Author

Thanks @huwshimi some very good points. Appreciate you took the time to write this up.

@petermakowski I know it's a bit awkward working with the external react-components dependency, but I wonder if it might be better to wait for the react-components PR to land and then do this work in a feature branch with a beta release of react-components?

I might've been more inclined to do this if it wasn't for the fact that the upgrade of react-components is blocked by vanilla 3.0 upgrade. This may not happen for a very long time (e.g. until we move over the final bits of Angular to React). Having feature branches sitting in the backlog has many downsides and I don't see any clear benefits of doing so.

Also, not sure if you've spotted this, but maas-ui has a way to link to react-components when developing:

https://github.com/canonical-web-and-design/maas-ui/blob/master/package.json#L32

Thanks! Yes I did know this, actually tested react-components a few times with other project's codebases to ensure it all works correctly when I was working on new react-components features.

@huwshimi
Copy link
Contributor

I might've been more inclined to do this if it wasn't for the fact that the upgrade of react-components is blocked by vanilla 3.0 upgrade. This may not happen for a very long time (e.g. until we move over the final bits of Angular to React). Having feature branches sitting in the backlog has many downsides and I don't see any clear benefits of doing so.

Oh, well I've got a present for you! The v3 PR is in review (it's actually been there for a couple of weeks):

#3647

@huwshimi
Copy link
Contributor

huwshimi commented Mar 15, 2022

@petermakowski
Copy link
Contributor Author

@huwshimi Opened another PR from a feature branch #3684

@petermakowski petermakowski deleted the add-modular-table branch March 16, 2022 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants