Fixes #36822 - Design new hosts overview page #9860

parthaa · 2023-10-10T18:10:15Z

Update the TableIndexPage as a part of this action to accept things like select all

Works with Katello/katello#10771

jeremylenz

It's finally here! 🥳

A couple things I noticed to start:

Search works, but autocomplete doesn't
Kebab is too far to the right

config/routes.rb

webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/Table.js

jeremylenz · 2023-10-13T13:04:43Z

webpack/assets/javascripts/react_app/components/HostsIndex/index.js

+
+  const customActionKebabs = [
+    {
+      title: __('Change content source'),


This should actually be in Katello since it's linking to a Katello page.

Should set this up similar to the way it's already done for host details.

See -> https://github.com/Katello/katello/blob/a57828483396b6e02e4b6d849202a0253abe9004/webpack/components/extensions/HostDetails/ActionsBar/index.js#L71

Exported from Katello to Foreman here -> https://github.com/Katello/katello/blob/a57828483396b6e02e4b6d849202a0253abe9004/webpack/global_index.js#L70

Consumed here in Foreman ->

foreman/webpack/assets/javascripts/react_app/components/HostDetails/ActionsBar/Selectors.js

Line 6 in d329c93

selectComponentByWeight('host-details-kebab');

which is then rendered here ->

foreman/webpack/assets/javascripts/react_app/components/HostDetails/ActionsBar/index.js

Line 49 in d329c93

const registeredItems = useSelector(selectKebabItems, shallowEqual);

jeremylenz · 2023-10-16T16:41:42Z

Also: Should add a setting "Use new All Hosts page" or some such, which will control whether the All Hosts link in the main nav goes to the new or the old page.

parthaa · 2023-10-16T21:05:13Z

Also: Should add a setting "Use new All Hosts page" or some such, which will control whether the All Hosts link in the main nav goes to the new or the old page.

Updated the menu

app/registries/foreman/settings/general.rb

webpack/assets/javascripts/react_app/components/HostsIndex/index.js

jeremylenz · 2023-10-18T20:28:05Z

webpack/assets/javascripts/react_app/components/HostsIndex/index.js

+
+  const registeredItems = useSelector(selectKebabItems, shallowEqual);
+  const customToolbarItems = (
+    <ForemanHostsIndexActionsBarContext.Provider value={{...selectAllOptions, fetchBulkParams}}>


MariaAga · 2023-10-23T15:11:03Z

webpack/assets/javascripts/react_app/routes/Models/ModelsPage/index.js

+  const response = useAPI(
+    'get',
+    `${MODELS_API_PATH}?include_permissions=true`,
+    {
+      key: API_REQUEST_KEY,
+    }
+  );
+


I didnt check the rest of the PR, but can we not move this out of the table? the reason for the table component is so other components will not handle api calls.
Also this change breaks webhooks which also use the table component (https://github.com/theforeman/foreman_webhooks/blob/master/webpack/ForemanWebhooks/Routes/Webhooks/WebhooksIndexPage/WebhooksIndexPage.js)

It made a lot more sense to me and @parthaa to move it up one level. This way the host-specifc parts (including the host API call) are in the host table component, while the generic table parts are in the table component. Not having access to response in the hosts table caused a lot of problems.

Perhaps we could leave it as is and update webhooks instead?

Cant the code use redux selectors to access the response?

That's a possibility, just seemed a bit less elegant to me.

Personally I feel like needing to access response thru Redux defeats the whole purpose of useAPI.

@parthaa thoughts?

Fixed it in the latest commit. Secret sauce here is this line

let response = useAPI( replacementResponse ? null : 'get', apiUrl.includes('include_permissions') ? apiUrl : `${apiUrl}?include_permissions=true`, { ...apiOptions, params: defaultParams, } );

method being null implies that useAPI effects wont do anything.

jeremylenz

some silly nits

app/registries/foreman/settings/general.rb

.../assets/javascripts/react_app/components/PF4/TableIndexPage/Table/SelectAllCheckbox/index.js

webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/TableHooks.js

MariaAga · 2023-10-24T13:36:25Z

webpack/assets/javascripts/react_app/components/HostsIndex/index.js

+      controller="hosts"
+      isDeleteable
+      columns={columns}
+      creatable={false}


Why is creatable false?

I think on the Hosts Index page we need create button as the same level as Hosts header. So decided to add that to the HostIndex component and not use TableIndex for that.

Will that be added in a different pr?

yeah, we'll be adding Delete right after this PR :)

And the create? 🙃

oops, misread that!

I can add create in the same PR, if partha doesn't add it here

webpack/assets/javascripts/react_app/components/HostsIndex/index.js

jeremylenz · 2023-10-24T16:26:19Z

app/helpers/application_helper.rb

@@ -415,6 +415,7 @@ def ui_settings
      labFeatures: Setting[:lab_features],
      safeMode: Setting[:safemode_render],
      displayFqdnForHosts: Setting[:display_fqdn_for_hosts],
+      displayNewHostsPage: Setting[:new_hosts_page]


I'd put this under User settings, since lab features is there too?

jeremylenz

Few things to wrap up -

Can we fix the kebab placement?
Can we suppress the "No results" message while the page is loading?
'Hosts' link should respect the setting every place it's used

MariaAga · 2023-10-25T08:28:20Z

@jeremylenz

Can we suppress the "No results" message while the page is loading?

why? its showing "no results - loading"

jeremylenz · 2023-10-25T13:48:58Z

why? its showing "no results - loading"

'No results' is a lie if it's still loading. We don't know the number of results until it loads.

anyway, we can fix this later, I'm not gonna block on this one :)

jeremylenz · 2023-10-25T14:49:52Z

Decided to leave for follow-up PR:

Multi-delete
Loading message (maybe)
Kebab placement

@parthaa can you please squash the commits and fix lint :)

jeremylenz

APJ. Let's get this in and continue on, since this will unblock everything else :)

thanks @parthaa!

parthaa · 2023-10-25T17:30:36Z

'No results' is a lie if it's still loading. We don't know the number of results until it loads.

anyway, we can fix this later, I'm not gonna block on this one :)

That logic is in TableIndexPage anyway. So once we make a decision all the pages using it should get updated appropriately.

jeremylenz

looks like Foreman and integration tests need a look!

Created a HostIndex component Update the TableIndexPage as a part of this action to accept things like select all Co-authored-by: Jeremy Lenz <[email protected]>

pr-processor bot added the Not yet reviewed label Oct 10, 2023

github-actions bot added the UI label Oct 10, 2023

parthaa force-pushed the my-changes branch 7 times, most recently from 1ab4260 to 4462bb4 Compare October 12, 2023 20:57

parthaa changed the title ~~[WIP] Fixes #36822 - Design new hosts page~~ Fixes #36822 - Design new hosts page Oct 12, 2023

jeremylenz reviewed Oct 12, 2023

View reviewed changes

config/routes.rb Outdated Show resolved Hide resolved

webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/Table.js Show resolved Hide resolved

ekohl changed the title ~~Fixes #36822 - Design new hosts page~~ Fixes #36822 - Design new hosts overview page Oct 13, 2023

jeremylenz reviewed Oct 13, 2023

View reviewed changes

parthaa force-pushed the my-changes branch from 4462bb4 to 1b1c644 Compare October 16, 2023 21:04

jeremylenz reviewed Oct 16, 2023

View reviewed changes

app/registries/foreman/settings/general.rb Outdated Show resolved Hide resolved

app/registries/foreman/settings/general.rb Outdated Show resolved Hide resolved

parthaa force-pushed the my-changes branch from c40b7ae to bc39865 Compare October 17, 2023 22:15

pr-processor bot added the Waiting on contributor label Oct 17, 2023

jeremylenz reviewed Oct 18, 2023

View reviewed changes

parthaa force-pushed the my-changes branch from e258c58 to 9bb092a Compare October 18, 2023 20:22

pr-processor bot removed the Waiting on contributor label Oct 18, 2023

jeremylenz reviewed Oct 18, 2023

View reviewed changes

parthaa mentioned this pull request Oct 18, 2023

Fixes #36862 - Added the slot fill for hosts index Katello/katello#10771

Merged

parthaa force-pushed the my-changes branch 6 times, most recently from 4df18f7 to 06ffbac Compare October 20, 2023 19:45

parthaa force-pushed the my-changes branch from 06ffbac to c96188d Compare October 21, 2023 06:37

MariaAga reviewed Oct 23, 2023

View reviewed changes

jeremylenz reviewed Oct 23, 2023

View reviewed changes

MariaAga reviewed Oct 24, 2023

View reviewed changes

webpack/assets/javascripts/react_app/components/HostsIndex/index.js Show resolved Hide resolved

jeremylenz reviewed Oct 24, 2023

View reviewed changes

parthaa force-pushed the my-changes branch 2 times, most recently from d005c3f to cc2bc38 Compare October 25, 2023 15:57

jeremylenz previously approved these changes Oct 25, 2023

View reviewed changes

pr-processor bot removed the Not yet reviewed label Oct 25, 2023

parthaa dismissed jeremylenz’s stale review via a3d6343 October 25, 2023 17:18

parthaa force-pushed the my-changes branch 2 times, most recently from a3d6343 to d9025c2 Compare October 25, 2023 17:21

jeremylenz approved these changes Oct 25, 2023

View reviewed changes

jeremylenz requested changes Oct 25, 2023

View reviewed changes

pr-processor bot added the Waiting on contributor label Oct 25, 2023

parthaa force-pushed the my-changes branch from d9025c2 to 1a7da6f Compare October 25, 2023 19:42

pr-processor bot added Needs re-review and removed Waiting on contributor labels Oct 25, 2023

Fixes #36822 - Design new hosts page

c8aea3e

Created a HostIndex component Update the TableIndexPage as a part of this action to accept things like select all Co-authored-by: Jeremy Lenz <[email protected]>

parthaa force-pushed the my-changes branch from 1a7da6f to c8aea3e Compare October 25, 2023 20:53

jeremylenz approved these changes Oct 26, 2023

View reviewed changes

pr-processor bot removed the Needs re-review label Oct 26, 2023

jeremylenz merged commit 3688a0d into theforeman:develop Oct 26, 2023
10 checks passed

jeremylenz mentioned this pull request Jan 2, 2024

Fixes #37018 - Use correct function signature for fetchBulkParams Katello/katello#10830

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #36822 - Design new hosts overview page #9860

Fixes #36822 - Design new hosts overview page #9860

parthaa commented Oct 10, 2023 •

edited

Loading

jeremylenz left a comment

jeremylenz Oct 13, 2023

jeremylenz Oct 13, 2023

parthaa Oct 18, 2023

jeremylenz commented Oct 16, 2023

parthaa commented Oct 16, 2023

jeremylenz Oct 18, 2023

MariaAga Oct 23, 2023

jeremylenz Oct 23, 2023

MariaAga Oct 23, 2023

jeremylenz Oct 23, 2023 •

edited

Loading

parthaa Oct 23, 2023

parthaa Oct 23, 2023

jeremylenz left a comment

MariaAga Oct 24, 2023

parthaa Oct 24, 2023

MariaAga Oct 24, 2023

jeremylenz Oct 24, 2023

MariaAga Oct 24, 2023

jeremylenz Oct 24, 2023

jeremylenz Oct 24, 2023

jeremylenz left a comment

MariaAga commented Oct 25, 2023

jeremylenz commented Oct 25, 2023

jeremylenz commented Oct 25, 2023

jeremylenz left a comment

parthaa commented Oct 25, 2023

jeremylenz left a comment

Fixes #36822 - Design new hosts overview page #9860

Fixes #36822 - Design new hosts overview page #9860

Conversation

parthaa commented Oct 10, 2023 • edited Loading

jeremylenz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeremylenz commented Oct 16, 2023

parthaa commented Oct 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeremylenz Oct 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeremylenz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeremylenz left a comment

Choose a reason for hiding this comment

MariaAga commented Oct 25, 2023

jeremylenz commented Oct 25, 2023

jeremylenz commented Oct 25, 2023

jeremylenz left a comment

Choose a reason for hiding this comment

parthaa commented Oct 25, 2023

jeremylenz left a comment

Choose a reason for hiding this comment

parthaa commented Oct 10, 2023 •

edited

Loading

jeremylenz Oct 23, 2023 •

edited

Loading