Make target selection same height as source selection in mapping form

MegaChriz → closed merge request !60

Gitpod url for testing https://gitpod.io#snapshot/a515d6c8-5fa6-4142-93cc-1c884c2b887d

I tested module using DrupalPod built with the fork in this issue, added import feed, imported content, tested with three different themes. I think that this is ready to be committed.

The only change that might be useful is to add is the following code for lines between rows.

#edit-mappings tr {
border-bottom: 1px solid #e6e4df;
}

I thought that it would be there by default, but some themes do not respect this property. So I am not sure if we need to add it to code or not really.

The last submitted patch, 10: feeds-mapping-extra-table-3173943-10.patch, failed testing. View results →

Adding the correct tag for Drupal Global Contribution Weekend 2023.

New form structure

mappings[0.url][map][select]
mappings[0.url][map][custom__xml][value]
edit-mappings-0value-configure -> rowspan
mappings[0.value][settings][format] -> rowspan
mappings[0.url][unique]
mappings[0.guid][unique]
remove_mappings[0] -> rowspan

Old form structure (better)

mappings[0][map][url][select]
mappings[0][map][url][custom__xml][value]
edit-mappings-2-configure -> rowspan
mappings[2][settings][format] -> rowspan
mappings[0][unique][url]
mappings[0][unique][guid]
remove_mappings[0] -> rowspan

@irinaz, @joelpittet discussed this issue at global contribution day.

On its own this new layout works nice, but it breaks Feeds Extensible Parsers. And that is because the structure of the mapping form is completely different.
@joelpittet is planning to take a look at the issue to see if he sees a way to get the same layout output, but with the original form structure preserved. This way we wouldn’t introduce a BC break. If we don’t see a way to do that, we’ll adjust Feeds Extensible Parser’s code and then commit this as is.

In #28 I've commented how the old form structure looks like compared to the new. Ideally, we would keep the old form structure, so Feeds Extensible Parsers can stay as it is. And also possible other modules that hook into the mapping form in a similar was as Feeds Extensible Parsers does.

@irinaz created a follow-up for this issue: 📌 Add class for the last row for each element in mapping Fixed

@joelpittet
The reason why delta and property are concatenated as $rows[$delta . '.' . $property] is because using $rows[$delta][$property] doesn't look like would work for rendering the table.

Using $rows[$delta . '.' . $property], the table array looks like this:

Using $rows[$delta][$property], the table array looks like this:

And the latter makes the mapping form look like this:

MegaChriz → credited joelpittet → .

I've used a different approach in the branch "3173943-row-per-property-after_build". As the name says, it provides a row per property but then in a way that doesn't break Feeds Extensible Parsers (I tested with that module + I ran all non-JS tests of it). Technical: the table build happens now in a #after_build callback.

I also added some CSS. We did add a separate ticket for that in 📌 Add class for the last row for each element in mapping Fixed , but while I was at this ticket I thought I could add that in as well. I'll remove it again in case it holds up getting this issue resolved.

This is how the mapping page then can look like:

I've added instructions about what to test for this issue.

@megachriz, I tested new branch on DrupalPod with address module, and it works great!!!

I checked the appearance in themes Bootstrap, Gin Admin Theme and Bootstrap5 and made a few changes to the CSS because of that. If tests still pass, I'll merge it!

And just as I was writing the above comment, tests finished. :D

Merged the code from branch 3173943-row-per-property-after_build.

Automatically closed - issue fixed for 2 weeks with no activity.