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

Automate Simulation Conversion Form from HAML to React #9202

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

liu-samuel
Copy link
Contributor

@liu-samuel liu-samuel commented Jun 6, 2024

Automation / Embedded Automate / Simulation

Converting the Automate Simulation Form from HAML to React and connecting the simulation tree summary to the form.

Old:
Screenshot 2024-06-13 at 11 30 00 AM
Screenshot 2024-06-13 at 11 29 39 AM

New:
Screenshot 2024-06-13 at 12 15 06 PM
Screenshot 2024-06-13 at 12 15 23 PM

@liu-samuel liu-samuel requested a review from a team as a code owner June 6, 2024 14:16
@liu-samuel liu-samuel force-pushed the automate-simulation-conversion branch 3 times, most recently from 9c83a53 to 9f28d96 Compare June 6, 2024 15:45
@liu-samuel liu-samuel force-pushed the automate-simulation-conversion branch 6 times, most recently from 3e87142 to 1ef52d3 Compare June 14, 2024 18:46
@jeffibm
Copy link
Member

jeffibm commented Jun 17, 2024

When we change the Type drop down, we get an error. could you check this please -
image

@jeffibm
Copy link
Member

jeffibm commented Jun 17, 2024

Could you make the title look the same if possible, like making them the same font size.
image

@jeffibm
Copy link
Member

jeffibm commented Jun 17, 2024

There is a failing test case under the GitHub actions/spec:routes. Could you please have a look at it..

app/stylesheet/automate-simulation.scss Outdated Show resolved Hide resolved
app/stylesheet/automate-simulation.scss Outdated Show resolved Hide resolved
app/views/layouts/_ae_resolve_options.html.haml Outdated Show resolved Hide resolved
app/stylesheet/automate-simulation.scss Outdated Show resolved Hide resolved
app/controllers/application_controller/automate.rb Outdated Show resolved Hide resolved
app/controllers/miq_ae_tools_controller.rb Outdated Show resolved Hide resolved
@liu-samuel
Copy link
Contributor Author

liu-samuel commented Jun 17, 2024

Could you make the title look the same if possible, like making them the same font size. image

Do you mean the "Simulation" text? I can't really see a difference between the old and new version

@liu-samuel liu-samuel force-pushed the automate-simulation-conversion branch 2 times, most recently from 4a00edf to bf9e80f Compare June 17, 2024 19:11
@GilbertCherrie
Copy link
Member

Leaving this as a reference for later.

To do:

  • Figure out what to do with the copy button (either remove the button or change the way it works)
  • Add the ability for the form to pre load values that were previously selected

@GilbertCherrie GilbertCherrie self-requested a review July 18, 2024 17:00
@liu-samuel liu-samuel force-pushed the automate-simulation-conversion branch 2 times, most recently from a97c32f to 81df64b Compare July 19, 2024 18:15
@miq-bot miq-bot added the stale label Nov 1, 2024
@miq-bot
Copy link
Member

miq-bot commented Nov 1, 2024

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s).

@GilbertCherrie
Copy link
Member

@liu-samuel can you please fix the tests here and also the old form has this flash message when the form is submitted:
Screenshot 2024-11-05 at 12 05 02 PM
can you please add this to your form too.

@GilbertCherrie
Copy link
Member

Also, the reset button also has a flash message that your form is missing.
Screenshot 2024-11-05 at 12 14 41 PM

Also when you reset the old form these are the values that the form gets reset to.
Screenshot 2024-11-05 at 12 15 28 PM

However, your form does not match this reset behaviour.
Screenshot 2024-11-05 at 12 16 12 PM

Can you please fix these issues too?

@liu-samuel liu-samuel force-pushed the automate-simulation-conversion branch 2 times, most recently from 9684e5e to e7618d9 Compare November 6, 2024 21:08
@liu-samuel liu-samuel force-pushed the automate-simulation-conversion branch 2 times, most recently from d2677b3 to 20b28a4 Compare November 13, 2024 16:41
Copy link
Member

@GilbertCherrie GilbertCherrie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please look into the comments I made

app/views/layouts/_ae_resolve_options.html.haml Outdated Show resolved Hide resolved
@liu-samuel liu-samuel force-pushed the automate-simulation-conversion branch 3 times, most recently from f9d17bf to c40a5dc Compare November 15, 2024 04:46
@miq-bot
Copy link
Member

miq-bot commented Nov 19, 2024

Checked commit liu-samuel@d560e28 with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
6 files checked, 38 offenses detected

app/controllers/miq_ae_tools_controller.rb

app/views/layouts/_ae_resolve_options.html.haml

  • ⚠️ - Line 4 - Layout/HashAlignment: Align the keys and values of a hash literal if they span more than one line.
  • ⚠️ - Line 5 - Layout/HashAlignment: Align the keys and values of a hash literal if they span more than one line.
  • ⚠️ - Line 6 - Layout/HashAlignment: Align the keys and values of a hash literal if they span more than one line.
  • ⚠️ - Line 7 - Layout/HashAlignment: Align the keys and values of a hash literal if they span more than one line.
  • ⚠️ - Line 7 - Line is too long. [88/80]
  • ⚠️ - Line 8 - Layout/HashAlignment: Align the keys and values of a hash literal if they span more than one line.

app/views/layouts/_content.html.haml

  • 💣 💥 🔥 🚒 - Line 15 - Do not repeat id "#left_div" on the page
  • 💣 💥 🔥 🚒 - Line 16 - Do not repeat id "#default_left_cell" on the page
  • 💣 💥 🔥 🚒 - Line 19 - Do not repeat id "#left_div" on the page
  • 💣 💥 🔥 🚒 - Line 20 - Do not repeat id "#default_left_cell" on the page
  • ⚠️ - Line 15 - Classes should be listed before IDs (.sidebar-pf should precede #left_div)
  • ⚠️ - Line 15 - id attribute must be in lisp-case
  • ⚠️ - Line 26 - Hash attribute should end with one space before the closing brace or be on its own line
  • ⚠️ - Line 26 - Hash attribute should start with one space after the opening brace
  • ⚠️ - Line 26 - Line is too long. [153/80]
  • ⚠️ - Line 27 - Classes should be listed before IDs (.row should precede #main-content)
  • ⚠️ - Line 31 - id attribute must be in lisp-case
  • ⚠️ - Line 32 - id attribute must be in lisp-case
  • ⚠️ - Line 34 - Line is too long. [110/80]
  • ⚠️ - Line 35 - Do not use inline style attributes
  • ⚠️ - Line 35 - Hash attribute should end with one space before the closing brace or be on its own line
  • ⚠️ - Line 35 - Hash attribute should start with one space after the opening brace
  • ⚠️ - Line 35 - id attribute must be in lisp-case
  • ⚠️ - Line 43 - Line is too long. [90/80]
  • ⚠️ - Line 44 - Line is too long. [89/80]
  • ⚠️ - Line 54 - Do not use inline style attributes
  • ⚠️ - Line 54 - Hash attribute should end with one space before the closing brace or be on its own line
  • ⚠️ - Line 54 - Hash attribute should start with one space after the opening brace
  • ⚠️ - Line 54 - Line is too long. [82/80]
  • ⚠️ - Line 54 - id attribute must be in lisp-case
  • ⚠️ - Line 56 - Avoid using instance variables in partials views
  • ⚠️ - Line 56 - Line is too long. [107/80]

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.

4 participants