UMB-CS-682-Team-03 / tracker

0 stars 0 forks source link

Make classhelper work when 'property' attribute is not set on the a tag. #35

Closed rouilj closed 5 months ago

rouilj commented 6 months ago

It is possible to use the original classhelper as an information panel and not a selector. See: https://issues.roundup-tracker.org/issue2551341 and click on Type or Components. These classhelper instances just show descriptive info so people can make the right choice.

The web component currently errors out if it is missing the property attribute in the URL. This crash happens in openPopUp when props.formProperty is referenced. This is easily fixable by using props?.formProperty instead. However there are a few more places that need to be fixed to allow the web component to work for this use case.

The things that have to change:

I think the search and pagination are fine.

I can see two ways to do this.

  1. put a "no-selection" class on the body of popup (or a data-noselection property ...). Then in css:
    1. hide the first column in the table using css
    2. hide the accumulator by setting display: none
    3. try to disable the click events on the table using pointer-events: none in some fashion. This won't disable return/space keyboard events.
  2. modify classhelper.js to disable the following when there is no property attribute defined
    1. do not add a click or keydown event handler for the selection table
    2. do not create the checkbox column for the header/footer/body of the table
    3. do not try to select preSelectedValues.includes(entry[headers[0]]) as preselectedValues might be nil. (alternatively make the setter of preselectedValues (maybe the setting of accumulatorValues in pageChange()) get an empty array/list.
      1. do not generate/append accumulatorFrag or the separator div above it

Because we want to remove click and other events and have to modify classhelper.js anyway, option 1 is not really an option.

I can see how to do most of this if I have access to the formProperty value from ConnectedCallback (or the fact that it is not set) in the other callbacks.

I got this to work by adding this.properties = properties after the try/catch block that sets properties. I was able to see it on the this variable while debugging in the other places. Not sure if this is safe/reliable but it seems to work.

rouilj commented 6 months ago

While I was looking at this, I found out that the classic tracker's helper doesn't work without a property attribute 8-). Good job on replicating the functionality of the server side classhelper down to a last bug 8-).

Using a classhelper without setting a property is apparently unique to the roundup issue tracker. So it's out of scope since it wasn't original functionality and it wasn't identified as a bug.

That being said, I have a fix for this using the classic tracker.

For this issue, a quick fix is to not register the classhelper if there is no property value. This will make it fall back to the server side help template which is fine.

patel-malav commented 5 months ago

This is should be resolved in 0cca0fe30f459c4b570aa35ed9a23ffd260f9aa0

Have tested this on local removing the property from tal classhelper link

rouilj commented 5 months ago

Something weird is happening. I reverted to revision 285f3406 (without any local patches).

In either chrome or firefox, with the priority header in issue.item.html changed to look like:

  <th class="required" i18n:translate="">Priority
    <roundup-classhelper data-search-with="name,order">
    <span tal:condition="context/is_edit_ok" tal:replace="structure python:db.priority.classhelp('id,name,order', pagesize=4)" />
  </roundup-classhelper>
</th>

I can click on the priority '(list)' and display the classhelper and it looks fine. I can't click on anything in the search element however. I can move the focus (and get an indicator) using tab/shift-tab. But clicking on any element of the search removes the focus indicator. But the focus stays where it was. Tabbing focuses on the element next to the element that had focus.

Ok, figured it out (finally noticed the cursor was not changing to an I beam over the inputs). The following CSS is breaking it.

.table-selection-none table {
    pointer-events: none;
}

I assume this was to prevent clicks on the selection table from firing an event or prevent the hover style from taking effect?

I enabled pointer events and I don't see an issue. Clicking does nothing but without a checkbox I don't think anybody will be confused. Hover highlights the row which is fine and makes it easier to scan.

I say just remove that rule. @Malav any reason to keep that rule?