Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closes #909 - Extend configuration UI to be able to search in the content of the configuration files #913

Merged
merged 9 commits into from
Aug 21, 2020

Conversation

mariusoe
Copy link
Member

@mariusoe mariusoe commented Aug 18, 2020

This change is Reviewable

Copy link
Contributor

@fylip97 fylip97 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 7 files reviewed, 5 unresolved discussions (waiting on @mariusoe)

a discussion (no related file):
Screenshot (19).png

If you open a file in a subfolder while searching, it might be good to open the subfolder in the filetree to see the marked file.



components/inspectit-ocelot-configurationserver-ui/src/components/views/configuration/ConfigurationView.js, line 118 at r1 (raw file):

  hideSearchDialog = () => this.setState({ isSearchDialogShown: false });

  openFile = (filename) => {

The name openFile could be confusing because the method is only called after the search and not every time a file is opened.
Maybe it should be renamed to openSeachedFile or something similar.


components/inspectit-ocelot-configurationserver-ui/src/components/views/configuration/dialogs/SearchDialog.js, line 78 at r1 (raw file):

              </span>
            ) : (
              match.text

Can you format the file.


components/inspectit-ocelot-configurationserver-ui/src/components/views/configuration/dialogs/SearchDialog.js, line 95 at r1 (raw file):

  /** The line number where the matches are starting */
  lineNumber: PropTypes.number,
  /** The file's content on the current lin */

Isn't lin missing an e?


components/inspectit-ocelot-configurationserver-ui/src/components/views/configuration/dialogs/SearchDialog.js, line 321 at r1 (raw file):

              />
            ) : (
              <span className="no-data">Nothing to show</span>

Please format the line.

Copy link
Member Author

@mariusoe mariusoe left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 7 files reviewed, 5 unresolved discussions (waiting on @fylip97)

a discussion (no related file):

Previously, fylip97 (Philip Dengler) wrote…

Screenshot (19).png

If you open a file in a subfolder while searching, it might be good to open the subfolder in the filetree to see the marked file.

This is done in a separate ticket



components/inspectit-ocelot-configurationserver-ui/src/components/views/configuration/ConfigurationView.js, line 118 at r1 (raw file):

Previously, fylip97 (Philip Dengler) wrote…

The name openFile could be confusing because the method is only called after the search and not every time a file is opened.
Maybe it should be renamed to openSeachedFile or something similar.

True, this is currently only used by the search dialog, but I would still keep it because acutually every component could use this for opening a file.


components/inspectit-ocelot-configurationserver-ui/src/components/views/configuration/dialogs/SearchDialog.js, line 78 at r1 (raw file):

Previously, fylip97 (Philip Dengler) wrote…

Can you format the file.

Hmm acutally, the formatting should be correct, otherwise the tests should fail as well.


components/inspectit-ocelot-configurationserver-ui/src/components/views/configuration/dialogs/SearchDialog.js, line 95 at r1 (raw file):

Previously, fylip97 (Philip Dengler) wrote…

Isn't lin missing an e?

Done


components/inspectit-ocelot-configurationserver-ui/src/components/views/configuration/dialogs/SearchDialog.js, line 321 at r1 (raw file):

he formatting should be correct, otherwise the tests should fail as well.
Here as well - the formatting should be correct, otherwise the tests should fail as well.

Copy link
Contributor

@fylip97 fylip97 left a comment

Choose a reason for hiding this comment

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

Reviewed 6 of 7 files at r1, 1 of 1 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@mariusoe mariusoe merged commit 43fe10a into inspectIT:master Aug 21, 2020
@mariusoe mariusoe deleted the feature/909-serach-view branch August 21, 2020 08:11
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extend configuration UI to be able to search in the content of the configuration files
2 participants