Skip to content
This repository has been archived by the owner on Jul 2, 2024. It is now read-only.

attachment-view.js: update toolbar default items to latest Lightning #12

Closed
wants to merge 2 commits into from

Conversation

Trim
Copy link
Member

@Trim Trim commented Jun 7, 2017

Fix javascript code to:

  • avoid removing event listener
  • avoid using try/catch without need
  • avoid to reset current set of items, but instead add back attachment
    if not already in toolbar and url attachment is there

Fix javascript code to:
 * avoid removing event listener
 * avoid using try/catch without need
 * avoid to reset current set of items, but instead add back attachment
   if not already in toolbar and url attachment is there
@Trim
Copy link
Member Author

Trim commented Jun 7, 2017

This PR should fix #9

It could need to reset the toolbar to default (see the personalize window) to have back real default buttons.

@advancingu
Copy link
Member

@Trim Thanks for the PR.

Is this how it is supposed to look for an Exchange calendar in the menu? Also, the Attachments button has no icon (but space is reserved for one).

Exchange calendar:
attach

For reference, here is a CalDAV calendar:
caldav

@advancingu advancingu changed the title attachment-view.js: update toolbar default items to latest Ligthning attachment-view.js: update toolbar default items to latest Lightning Jun 7, 2017
@Trim
Copy link
Member Author

Trim commented Jun 8, 2017

In the xul, there's no image defined, that's why we don't see one. I'll check if I can add the same image than the standard button.

For the menu, it should not display a list, but open the dialog too.

Thanks for your tests !

@Trim
Copy link
Member Author

Trim commented Jun 8, 2017

I have to close this PR, because here I have the same issue than the editor in event dialog:

Now, the event dialog window is broken, because all items have been moved in an iframe by Lightning.

I'm working on it and I add the commits to the PR #11

@Trim Trim closed this Jun 8, 2017
@advancingu
Copy link
Member

@Trim In this case, I would suggest fixing #11 first and once that is merged work on the fix for this issue here separately. It makes it much easier to review PRs when only one issue is addressed at a time. However, if you already have the code included in the other branch now, leave it in there.

@Trim
Copy link
Member Author

Trim commented Jun 8, 2017

Yes I understand it's better to do one by one, but the changes made by Lightning forces me to do everything at same time: the new iframe is not accessible from the window and the window is not accessible from the iframe.

So I have to find a way to do actions from menus and from dialog even if the DOM space is not shared. I'm taking time tomorrow to study how Lightning is able to do that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants