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

[dev] Regressions on produce screen and forms layout #1381

Closed
AlexisSouquiere opened this issue Feb 15, 2023 · 7 comments
Closed

[dev] Regressions on produce screen and forms layout #1381

AlexisSouquiere opened this issue Feb 15, 2023 · 7 comments

Comments

@AlexisSouquiere
Copy link
Collaborator

When I'm running to dev branch locally I'm facing somes issues that might be regressions since 0.23.0

  • Dropdowns on the produce to topic screen seems to be broken due to Import lodash functions explicitily #1335. The unused "options" parameter in renderDropdown has been removed but this parameter was sent in TopicProduce and it brokes the dropdowns on this page
    image

  • Multi / Tombstone checkboxes seems to be broken too due to Fix eslint warnings  #1254. When I click on it, nothing happens. There is a extra 2nd parameter given in the renderCheckbox that shifts all the other parameters and cause the issue
    image

  • Globaly the forms layout changed and some inputs are not displayed on the same line as the label
    image
    image

Am I the only one having the issue ? I just want to be sure of it (because it's frontend 😅) before creating a PR for this branch AlexisSouquiere@123870c.

I saw that one of my change is reverting one of your change @tchiotludo (9e2ee4a) so it's kind of strange

cc @lucapette as you seems to be the fronted expert !

@AlexisSouquiere AlexisSouquiere changed the title [dev] Regressions on produce form and forms layout [dev] Regressions on produce screen and forms layout Feb 15, 2023
@tchiotludo
Copy link
Owner

@lucapette do you will have time to look or not? This one is blocking a release

@lucapette
Copy link
Contributor

@tchiotludo @AlexisSouquiere oh my. No idea how I missed this. I'll have a look now :)

@AlexisSouquiere
Copy link
Collaborator Author

AlexisSouquiere commented Mar 6, 2023

@lucapette if you can reproduce the same issue then you can check my branch AlexisSouquiere@123870c where I fixed it. If you think that its the right fix I'll create the PR

@lucapette
Copy link
Contributor

@AlexisSouquiere 👏 amazing work!

I'm not sure how I didn't catch this at the time I did the prs you mentioned. On one side, it was a lot work, on the other I tested the produce page every time... anyway, thanks to you, it's going to be fixed soon.

I'd open a pr as is. I thought I had changed the way we do default classes on input too but apparently I only did it on Select (something like <div className={${wrapperClass || 'form-group row'}}>) so if you want you can also do that (the code looks a little better I think but I can take care of it in a subsequent pr).

@tchiotludo Feb was a pretty busy month for me but, from the look of it, I may have some time in the coming weeks. I took lots of notes about a potential react upgrade. Should I open an issue so we make a plan? (no strings attached)

@tchiotludo
Copy link
Owner

@lucapette yes tracking in an issue or draft PR can be a good start (and no worries, we all have things that pay the bill to do 😄)

@lucapette
Copy link
Contributor

@tchiotludo I'll open an issue with a plan draft soon. It's quite some work but I think the end result would be pretty good for the frontend code

@AlexisSouquiere
Copy link
Collaborator Author

@lucapette thanks :) I won't have time to work on it today so I create the PR as it but yes we can still clean it in a future PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants