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

Migrate CIDER Resource Allocation Interface to XRAS Admin and implement relative_order drag and sort functionality #8

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

asimregmi
Copy link
Collaborator

Overview

This PR implements the migration of "Allocations" page from CIDER into XRAS Admin resources/resource_id page. It also adds a new dragging and sorting functionality in the main /resources listing page.

Branch ramps-583-implement-relative-order-column-in-allocations_process_resources is checked out from ramps-554-migrate-cider-resource-allocations-interface so has both changes from the two Jira tickets.

Related

Changes

  • added EditResource.jsx and supporting React components, helpers, and reducers inside /src/edit-resource
  • added Resources.jsx and supporting reducers and helper functions inside /src/resources
  • added new shared components and made changes to existing shared components Grid.jsx, Alert.jsx
  • See also: updated 'Save Resource' button logic in xras_admin/app/views/resources/show.html.erb
  • change in app/views/resources/index.html.erb and app/views/resources/show.html.erb to accomodate new React components

Testing

  1. git pull origin/ramps-583-implement-relative-order-column-in-allocations_process_resources in xras_admin repo
  2. git pull origin/ramps-583-implement-relative-order-column-in-allocations_process_resources in xras-ui repo
  3. Add a new relative_order integer column in xras.allocations_process_resources table
  4. npm run dev in xras-ui directory

@asimregmi asimregmi changed the title Migrate CIDER Resource Allocation Interface to XRAS Admin and implement relative_order drag and sort functionality RAMPS-554 & RAMPS-583 - Migrate CIDER Resource Allocation Interface to XRAS Admin and implement relative_order drag and sort functionality Oct 23, 2024
@asimregmi asimregmi changed the title RAMPS-554 & RAMPS-583 - Migrate CIDER Resource Allocation Interface to XRAS Admin and implement relative_order drag and sort functionality Migrate CIDER Resource Allocation Interface to XRAS Admin and implement relative_order drag and sort functionality Oct 23, 2024
Copy link
Collaborator

@yomatters yomatters left a comment

Choose a reason for hiding this comment

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

@asimregmi, this is looking really good! I've made some specific suggestions about parts of the code inline. Here are a few general points of feedback, mainly about the user experience.

  • On the resource list page, please remove the paragraph beginning with "Only resource submission questions can be modified...". That's not true anymore with these changes.
  • On the resource edit page, I suggest renaming the "Descriptive Text" column to "Help Text." I think that will help to distinguish it from the "Allocations Description" field.
  • I'd recommend changing the names of the buttons above the Allocations Types tables to "Manage Allocation Types" and "Manage Required Resources." Then in the modal, you can display a list of checkboxes instead of a select field. That will also allow users to remove an item, which I think is particularly important for the required resources.
  • With Vite, you don't need to import the global React object in each component unless you're explicitly referencing it.
  • I'd recommend using a code formatter and linter to format and check your JavaScript code. I like Prettier for formatting and ESLint for linting. Both have extensions that integrate them with VSCode (and probably other editors), or you can run them from the command line.

src/edit-resource/AddNewModal.jsx Outdated Show resolved Hide resolved
src/edit-resource/EditResource.jsx Outdated Show resolved Hide resolved
src/shared/Checkbox/CheckboxGroup.jsx Outdated Show resolved Hide resolved
src/edit-resource/EditResource.jsx Outdated Show resolved Hide resolved
src/resources/helpers/utils.js Outdated Show resolved Hide resolved
src/shared/Form/FormField.jsx Outdated Show resolved Hide resolved
Comment on lines 10 to 40
select: ({ column, row, style }) => (
<td style={style}>
<SelectInput
label=""
options={row[column.key].options}
value={row[column.key].value}
onChange={(e) => row[column.key].onChange(e.target.value)}
style={{ width: '100%', margin: 0 }}
/>
</td>
),
input: ({ column, row, style }) => (
<td style={style}>
<FormField
label=""
type="text"
value={row[column.key].value}
onChange={(e) => row[column.key].onChange(e.target.value)}
style={{ width: '92%', margin: 0 }}
/>
</td>
),
checkbox: ({ column, row, style }) => (
<td style={style}>
<input
type="checkbox"
checked={row[column.key].checked}
onChange={(e) => row[column.key].onChange(e.target.checked)}
/>
</td>
),
Copy link
Collaborator

Choose a reason for hiding this comment

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

This approach, where the row contains configuration objects for each field, is interesting, though it's different from what I did in the My Projects interface. Let's discuss this offline and decide how we're going to handle editable fields in the grid so that we can use the same approach everywhere.

const Tooltip = ({ title, placement, children }) => {
// useEffect hook to initialize the tooltip on component mount & destroy on component unmount
useEffect(() => {
$('[data-toggle="tooltip"]').tooltip();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd prefer not to add a jQuery dependency if we can avoid it, though I realize that may be tricky with Bootstrap 2. Have you tried the Tooltip component from React Bootstrap? If the CSS classes haven't changed, it may work with Bootstrap 2 as well.

Comment on lines 16 to 49
const handleDragStart = (e, index) => {
draggedIndexRef.current = index;
};

const handleDragOver = (e, index) => {
e.preventDefault();
if (draggedIndexRef.current === index) return;

const newResources = [...resources];
const draggedItem = newResources[draggedIndexRef.current];
newResources.splice(draggedIndexRef.current, 1);
newResources.splice(index, 0, draggedItem);

draggedIndexRef.current = index;
setResources(newResources);

// Scroll logic to start scroll scroll when dragging items
const { clientY } = e;
const scrollThreshold = 130;

if (clientY < scrollThreshold) {
startScrolling(-1, scrollIntervalRef);
} else if (window.innerHeight - clientY < scrollThreshold) {
startScrolling(1, scrollIntervalRef);
} else {
stopScrolling(scrollIntervalRef);
}
};

const handleDrop = () => {
draggedIndexRef.current = null;
stopScrolling(scrollIntervalRef);
updateBackend(relative_url_root, resources);
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

If it's not too much work, it might be nice to break the drag-to-reorder logic out into a separate component. I can imagine it's something we might want to use elsewhere.

src/shared/SelectInput/SelectInput.jsx Outdated Show resolved Hide resolved
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

Successfully merging this pull request may close these issues.

3 participants