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

[FEATURE] serveIndex: use serve-index for serving the application index #253

Merged
merged 2 commits into from
Oct 31, 2019
Merged

[FEATURE] serveIndex: use serve-index for serving the application index #253

merged 2 commits into from
Oct 31, 2019

Conversation

jkoenig134
Copy link
Contributor

My changes

  • using https://www.npmjs.com/package/serve-index for serving index files
  • Index is now more user-friendly
  • implements possibility for @ui5/cli to show hidden files and choose a view (tiles or details)
    createMiddleware has now therefore 3 arguments (the new one are optional -> no beaking changes
    createMiddleware({resources, tiles, showHidden})
    • the resources object stays the same
    • the tiles object boolean declares if the ui should show tiles or a detailed table
    • the showHidden boolean declares if dot files should be hidden or not

Pull Request Checklist

- using https://www.npmjs.com/package/serve-index for serving index files
- Index is now more user-friendly
- implements possibility for @ui5/cli to show hidden files and choose a view (tiles or details)
@CLAassistant
Copy link

CLAassistant commented Oct 29, 2019

CLA assistant check
All committers have signed the CLA.

- date checking fails because of different formatting
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.9%) to 91.656% when pulling 833b83f on jkoenig134:master into d6cf026 on SAP:master.

@RandomByte RandomByte added the enhancement New feature or request label Oct 29, 2019
@RandomByte RandomByte self-requested a review October 29, 2019 18:55
@RandomByte
Copy link
Member

Hey @jkoenig134, thank you for this PR! 🙌

Since serve-index is licensed under MIT, we need to include the license and properly attribute the code to the serve-index project.

We'll do both in a follow up PR since we might need to do some minor refactoring to better separate the original serve-index code from ours.

@RandomByte RandomByte merged commit d6ea507 into SAP:master Oct 31, 2019
@RandomByte
Copy link
Member

For future reference, a related PR: expressjs/serve-index#67

@jkoenig134
Copy link
Contributor Author

Hey @RandomByte, thank you very much for approval and merging!
I will add a PR at @ui5/cli soon, for integration of the tiles and hidden functionalities.

@RandomByte
Copy link
Member

Cool, looking forward to it! Just to let you know, in #254 we swapped the API from tiles to showDetails (defaulting to true). We are thinking of reworking the HTML template somewhere in the future, maybe using UI5 Web Components 🤔. So better to keep it generic.

Are you thinking of exposing these options as CLI parameters?

@jkoenig134
Copy link
Contributor Author

@RandomByte the options are beeing exposed in my new pull request for @ui5/server, upcoming in the next minutes. I TOTALLY forgot this in the last PR...

RandomByte pushed a commit to SAP/ui5-cli that referenced this pull request Nov 7, 2019
The directory listing of the UI5 Server has been overhauled by
integrating parts of the serve-index[1] project (see [2]).

The '--simple-index' flag allows to toggle between a detailed- and a
simplified rendering of the directory index.

Thanks to @jkoenig134 for this contribution!

[1] https://github.com/expressjs/serve-index
[2] SAP/ui5-server#253
RandomByte pushed a commit to SAP/ui5-cli that referenced this pull request Nov 7, 2019
The directory listing of the UI5 Server has been overhauled by
integrating parts of the serve-index[1] project (see [2]).

The '--simple-index' flag allows to toggle between a detailed- and a
simplified rendering of the directory index.

Thanks to @jkoenig134 for this contribution!

[1] https://github.com/expressjs/serve-index
[2] SAP/ui5-server#253
@RandomByte
Copy link
Member

Released as part of UI5 CLI v1.12.0 🎉

@jkoenig134
Copy link
Contributor Author

🎉🎉🎉🎉

@vobu
Copy link

vobu commented Nov 7, 2019

i have talented colleagues, yeah

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants