Skip to content

Add newFileMenu and refactor library with rollup #420

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.

Sign up for GitHub

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

Jump to bottom
Merged
skjnldsv merged 2 commits into from
Jun 16, 2022
Merged

Add newFileMenu and refactor library with rollup #420

skjnldsv merged 2 commits into from
Jun 16, 2022

Conversation

@skjnldsv
Copy link
Contributor

@skjnldsv skjnldsv commented Jun 9, 2022 •
edited
Loading

@nextcloud/server

skjnldsv
skjnldsv commented Jun 9, 2022
View reviewed changes
@skjnldsv skjnldsv changed the title Add newFileMenu and refactor library Add newFileMenu and refactor library with rollup Jun 9, 2022
@skjnldsv skjnldsv force-pushed the feat/newFileMenu-refactor branch from a168483 to b78a6fc Compare June 9, 2022 10:53
@skjnldsv skjnldsv added the type: enhancement 🚀 New feature or request label Jun 9, 2022
@codecov

This comment was marked as resolved.

Sign in to view
artonge
artonge requested changes Jun 14, 2022
View reviewed changes
@skjnldsv skjnldsv requested a review from artonge June 14, 2022 16:15
@skjnldsv skjnldsv force-pushed the feat/newFileMenu-refactor branch from b78a6fc to 8e2dee6 Compare June 14, 2022 16:16
ChristophWurst
ChristophWurst reviewed Jun 15, 2022
View reviewed changes
ChristophWurst
ChristophWurst reviewed Jun 15, 2022
View reviewed changes
Copy link
Contributor

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Merge this PR if you must, but then I'm fully removing myself from this package

@ChristophWurst
Copy link
Contributor

ChristophWurst commented Jun 15, 2022

Is there a ticket we could reference for anyone who is not familiar with newFileMenu?

CarlSchwan
CarlSchwan reviewed Jun 15, 2022
View reviewed changes
@skjnldsv skjnldsv force-pushed the feat/newFileMenu-refactor branch 4 times, most recently from 9ae7077 to 27f31f7 Compare June 15, 2022 17:53
@skjnldsv
Copy link
Contributor Author

skjnldsv commented Jun 15, 2022

Here we go, I removed as much as unrelated things I could find @ChristophWurst
I will create other PR for the workflows, and maybe another one for babel if I have time :)

CarlSchwan
CarlSchwan reviewed Jun 15, 2022
View reviewed changes
@skjnldsv skjnldsv force-pushed the feat/newFileMenu-refactor branch from 27f31f7 to 7c1cd70 Compare June 16, 2022 05:54
@skjnldsv skjnldsv requested a review from CarlSchwan June 16, 2022 05:54
@skjnldsv
Better documentation
bfeb49c 
Signed-off-by: John Molakvoæ <[email protected]>
@skjnldsv skjnldsv merged commit 32af081 into master Jun 16, 2022
@skjnldsv skjnldsv deleted the feat/newFileMenu-refactor branch June 16, 2022 16:53
skjnldsv
skjnldsv commented Jun 17, 2022
View reviewed changes
lib/newFileMenu.ts
iconSvgInline?: string
/** Existing icon css class */
iconClass?: string
fileType: FileType
Copy link
Contributor Author

@skjnldsv skjnldsv Jun 17, 2022

Choose a reason for hiding this comment

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

@juliushaertl @PVince81 any of you know why we actually use that property?
It has been like that for years
https://github.com/nextcloud/server/blob/ac2bc2384efe3c15ff987b87a7432bc60d545c67/apps/files/js/newfilemenu.js#L204
https://github.com/nextcloud/server/blob/ac2bc2384efe3c15ff987b87a7432bc60d545c67/apps/files/js/newfilemenu.js#L110

Copy link
Contributor Author

@skjnldsv skjnldsv Jun 17, 2022

Choose a reason for hiding this comment

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

Seems to be a leftover from how we used to manage creation
owncloud/core@d386168#diff-4e16996e525fd1d3ef643308e0c2edbd87c0776afb54b426b3a21cad6fca1868L181

Copy link
Contributor Author

@skjnldsv skjnldsv Jun 17, 2022

Choose a reason for hiding this comment

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

I guess we can drop this.
The only thing we need is icon, text, new file text string, ID and handler, no?

ChristophWurst
ChristophWurst reviewed Sep 27, 2022
View reviewed changes
jest.config.js
},
},
transformIgnorePatterns: [
'node_modules/(?!(abcde|fghij)/)',
Copy link
Contributor

@ChristophWurst ChristophWurst Sep 27, 2022

Choose a reason for hiding this comment

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

🧐

@skjnldsv skjnldsv mentioned this pull request Nov 8, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ChristophWurst ChristophWurst ChristophWurst left review comments

@blizzz blizzz blizzz approved these changes

@artonge artonge artonge approved these changes

@szaimen szaimen Awaiting requested review from szaimen

@CarlSchwan CarlSchwan Awaiting requested review from CarlSchwan

Assignees

No one assigned

Labels

type: enhancement 🚀 New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@skjnldsv @ChristophWurst @blizzz @artonge @CarlSchwan